Thread (10 messages) 10 messages, 3 authors, 2020-10-21

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

From: Aditya <hidden>
Date: 2020-10-21 12:10:03

On 21/10/20 2:22 pm, Lukas Bulwahn wrote:

On Wed, 21 Oct 2020, Dwaipayan Ray wrote:
quoted
Hey Aditya and Lukas,
quoted
quoted
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help