Re: [Linux-kernel-mentees] [PATCH] checkpatch: fix false positive for REPEATED_WORD warning

From: Aditya <yashsri421@gmail.com>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>,
	Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: linux-kernel-mentees@lists.linuxfoundation.org
Subject: Re: [Linux-kernel-mentees] [PATCH] checkpatch: fix false positive for REPEATED_WORD warning
Date: Wed, 21 Oct 2020 17:39:50 +0530	[thread overview]
Message-ID: <81e9cdec-d3c7-e5ba-0f2d-061fc0738385@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.21.2010211049020.8328@felia>

On 21/10/20 2:22 pm, Lukas Bulwahn wrote:
> 
> 
> On Wed, 21 Oct 2020, Dwaipayan Ray wrote:
> 
>> Hey Aditya and Lukas,
>>
>>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>>>> index 9b9ffd876e8a..181c95691715 100755
>>>>> --- a/scripts/checkpatch.pl
>>>>> +++ b/scripts/checkpatch.pl
>>>>> @@ -3052,7 +3052,9 @@ sub process {
>>>>>
>>>>>  # check for repeated words separated by a single space
>>>>>             if ($rawline =~ /^\+/ || $in_commit_log) {
>>>>> -                   while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
>>>>> +                   # avoid repeating hex occurrences like 'ff ff fe 09 ...'
>>>>> +                   while ($rawline !~ /((\s)*[0-9a-z]{2}( )+){4,}/ &&
>>
>> Pattern is probably wrong. It doesn't recognize word boundaries or
>> tabs between words. Example of the first type:
>>
>> 000 00 ff ff ...
>>
> 
> I am wondering if this pattern really appears.
> 
> Hex stuff is usually written two-letter and spaces.
> 
> Maybe it is best to limit it to 0-9a-f, though. I think there should not 
> be matches with other letters than that.
> 
> Aditya, evaluations on those alternatives would help to make decisions.
> 
>> The regex matches "00 00 ff ff" ignoring the first 0.
>>
>> I think it could be perhaps better with something like:
>>
>>  # check for repeated words separated by a single space
>> -               if ($rawline =~ /^\+/ || $in_commit_log) {
>> +               if (($rawline =~ /^\+/ || $in_commit_log) &&
>> +                   $rawline !~ /(?:\b(?:[0-9a-f]{2}\s+){4,})/) {
>>                         pos($rawline) = 1 if (!$in_commit_log);
>>                         while ($rawline =~ /\b($word_pattern)
>> (?=($word_pattern))/g) {
>>
>> Please test it though. I only ran it on a few patterns.
>>
>> Apart from it, this does fix the problem. But I am quite sceptical about
>> matching 4 or more 2 lettered words in a row. There could be counter
>> examples but I guess that is very rare. It's not very general, but for
>> the moment it does the job.
>>
>> So I think it's probably good with some changes. Not sure what Joe
>> would have in mind though.
>>
>> Lukas, I think with the changes in place, it is ready to go for discussion.
>>
> 
> Dwaipayan, thanks for your review.
> 
> Lukas
> 

Hi Sir
I made these changes:
 # check for repeated words separated by a single space
 		if ($rawline =~ /^\+/ || $in_commit_log) {
-			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
+			# avoid repeating hex occurrences like 'ff ff fe 09 ...'
+			while ($rawline !~ /(\b[0-9a-f]{2}( )+){4,}/ &&
+				$rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {

 				my $first = $1;
 				my $second = $2;


Reports:
List of errors and warnings after applying the patch:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task3/summary.txt

Change in errors and warnings compared to previous patch:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task3/relative_summary/summary_relative.txt

Dropped warnings compared to previous patch:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task3/relative_summary/dropped_warnings/summary.txt

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees