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

[Lukas Bulwahn]

On Wed, 21 Oct 2020, Aditya wrote:

> 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
>

Looks good.

I suggest you quickly scan through the dropped warnings and confirm, so 
that you can add something like this to your commit message:

---
A quick evaluation on <git commit range, fill in here> showed that
this change reduces REPEATED_WORD warnings from xxx to yyy.

A quick manual check found all cases are related to hex output in commit 
messages.

---

Then send out the patch again here quickly and if we do not see big 
mistake, send it our to lkml and Joe Perches.

If you need any help, just let us know.

Lukas