All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
@ 2020-10-24  0:08 ` Aditya Srivastava
  0 siblings, 0 replies; 8+ messages in thread
From: Aditya Srivastava @ 2020-10-24  0:08 UTC (permalink / raw)
  To: joe
  Cc: linux-kernel, linux-kernel-mentees, lukas.bulwahn, dwaipayanray1,
	Aditya Srivastava

Presence of hexadecimal address or symbol results in false warning
message by checkpatch.pl.

For example, running checkpatch on commit b8ad540dd4e4 ("mptcp: fix
memory leak in mptcp_subflow_create_socket()") results in warning:

WARNING:REPEATED_WORD: Possible repeated word: 'ff'
    00 00 00 00 00 00 00 00 00 2f 30 0a 81 88 ff ff  ........./0.....

Similarly, the presence of list command output in commit results in
an unnecessary warning.

For example, running checkpatch on commit 899e5ffbf246 ("perf record:
Introduce --switch-output-event") gives:

WARNING:REPEATED_WORD: Possible repeated word: 'root'
  dr-xr-x---. 12 root root    4096 Apr 27 17:46 ..

Here, it reports 'ff' and 'root to be repeated, but it is in fact part
of some address or code, where it has to be repeated.

In these cases, the intent of the warning to find stylistic issues in
commit messages is not met and the warning is just completely wrong in
this case.

To avoid these warnings, add additional regex check for the
directory permission pattern and avoid checking the line for this
class of warning. Similarly, to avoid hex pattern, check if the word
consists of hex symbols and skip this warning if it is not among the
common english words formed using hex letters.

A quick evaluation on v5.6..v5.8 showed that this fix reduces
REPEATED_WORD warnings from 2797 to 907.

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

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
 scripts/checkpatch.pl | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7e505688257a..1d42d08d520b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3049,7 +3049,9 @@ sub process {
 		}
 
 # check for repeated words separated by a single space
-		if ($rawline =~ /^\+/ || $in_commit_log) {
+# avoid false positive from list command eg, '-rw-r--r-- 1 root root'
+		if (($rawline =~ /^\+/ || $in_commit_log) &&
+                    $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) {
 			pos($rawline) = 1 if (!$in_commit_log);
 			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
 
@@ -3074,6 +3076,17 @@ sub process {
 				next if ($start_char =~ /^\S$/);
 				next if (index(" \t.,;?!", $end_char) == -1);
 
+                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
+                                my %allow_repeated_words = (
+                                        add => '',
+                                        added => '',
+                                        bad => '',
+                                        be => '',
+                                );
+                                if ($first =~ /\b[0-9a-f]{2,}\b/) {
+                                        next if (!exists($allow_repeated_words{lc($first)}));
+                                }
+
 				if (WARN("REPEATED_WORD",
 					 "Possible repeated word: '$first'\n" . $herecurr) &&
 				    $fix) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [Linux-kernel-mentees] [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
@ 2020-10-24  0:08 ` Aditya Srivastava
  0 siblings, 0 replies; 8+ messages in thread
From: Aditya Srivastava @ 2020-10-24  0:08 UTC (permalink / raw)
  To: joe; +Cc: Aditya Srivastava, linux-kernel-mentees, linux-kernel, dwaipayanray1

Presence of hexadecimal address or symbol results in false warning
message by checkpatch.pl.

For example, running checkpatch on commit b8ad540dd4e4 ("mptcp: fix
memory leak in mptcp_subflow_create_socket()") results in warning:

WARNING:REPEATED_WORD: Possible repeated word: 'ff'
    00 00 00 00 00 00 00 00 00 2f 30 0a 81 88 ff ff  ........./0.....

Similarly, the presence of list command output in commit results in
an unnecessary warning.

For example, running checkpatch on commit 899e5ffbf246 ("perf record:
Introduce --switch-output-event") gives:

WARNING:REPEATED_WORD: Possible repeated word: 'root'
  dr-xr-x---. 12 root root    4096 Apr 27 17:46 ..

Here, it reports 'ff' and 'root to be repeated, but it is in fact part
of some address or code, where it has to be repeated.

In these cases, the intent of the warning to find stylistic issues in
commit messages is not met and the warning is just completely wrong in
this case.

To avoid these warnings, add additional regex check for the
directory permission pattern and avoid checking the line for this
class of warning. Similarly, to avoid hex pattern, check if the word
consists of hex symbols and skip this warning if it is not among the
common english words formed using hex letters.

A quick evaluation on v5.6..v5.8 showed that this fix reduces
REPEATED_WORD warnings from 2797 to 907.

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

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
 scripts/checkpatch.pl | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7e505688257a..1d42d08d520b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3049,7 +3049,9 @@ sub process {
 		}
 
 # check for repeated words separated by a single space
-		if ($rawline =~ /^\+/ || $in_commit_log) {
+# avoid false positive from list command eg, '-rw-r--r-- 1 root root'
+		if (($rawline =~ /^\+/ || $in_commit_log) &&
+                    $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) {
 			pos($rawline) = 1 if (!$in_commit_log);
 			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
 
@@ -3074,6 +3076,17 @@ sub process {
 				next if ($start_char =~ /^\S$/);
 				next if (index(" \t.,;?!", $end_char) == -1);
 
+                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
+                                my %allow_repeated_words = (
+                                        add => '',
+                                        added => '',
+                                        bad => '',
+                                        be => '',
+                                );
+                                if ($first =~ /\b[0-9a-f]{2,}\b/) {
+                                        next if (!exists($allow_repeated_words{lc($first)}));
+                                }
+
 				if (WARN("REPEATED_WORD",
 					 "Possible repeated word: '$first'\n" . $herecurr) &&
 				    $fix) {
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
  2020-10-24  0:08 ` [Linux-kernel-mentees] " Aditya Srivastava
@ 2020-10-24  0:27   ` Aditya
  -1 siblings, 0 replies; 8+ messages in thread
From: Aditya @ 2020-10-24  0:27 UTC (permalink / raw)
  To: joe; +Cc: linux-kernel, linux-kernel-mentees, lukas.bulwahn, dwaipayanray1

On 24/10/20 5:38 am, Aditya Srivastava wrote:
> Presence of hexadecimal address or symbol results in false warning
> message by checkpatch.pl.
> 
> For example, running checkpatch on commit b8ad540dd4e4 ("mptcp: fix
> memory leak in mptcp_subflow_create_socket()") results in warning:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'ff'
>     00 00 00 00 00 00 00 00 00 2f 30 0a 81 88 ff ff  ........./0.....
> 
> Similarly, the presence of list command output in commit results in
> an unnecessary warning.
> 
> For example, running checkpatch on commit 899e5ffbf246 ("perf record:
> Introduce --switch-output-event") gives:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'root'
>   dr-xr-x---. 12 root root    4096 Apr 27 17:46 ..
> 
> Here, it reports 'ff' and 'root to be repeated, but it is in fact part
> of some address or code, where it has to be repeated.
> 
> In these cases, the intent of the warning to find stylistic issues in
> commit messages is not met and the warning is just completely wrong in
> this case.
> 
> To avoid these warnings, add additional regex check for the
> directory permission pattern and avoid checking the line for this
> class of warning. Similarly, to avoid hex pattern, check if the word
> consists of hex symbols and skip this warning if it is not among the
> common english words formed using hex letters.
> 
> A quick evaluation on v5.6..v5.8 showed that this fix reduces
> REPEATED_WORD warnings from 2797 to 907.
> 
> A quick manual check found all cases are related to hex output or
> list command outputs in commit messages.
> 
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
>  scripts/checkpatch.pl | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7e505688257a..1d42d08d520b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3049,7 +3049,9 @@ sub process {
>  		}
>  
>  # check for repeated words separated by a single space
> -		if ($rawline =~ /^\+/ || $in_commit_log) {
> +# avoid false positive from list command eg, '-rw-r--r-- 1 root root'
> +		if (($rawline =~ /^\+/ || $in_commit_log) &&
> +                    $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) {
>  			pos($rawline) = 1 if (!$in_commit_log);
>  			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
>  
> @@ -3074,6 +3076,17 @@ sub process {
>  				next if ($start_char =~ /^\S$/);
>  				next if (index(" \t.,;?!", $end_char) == -1);
>  
> +                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
> +                                my %allow_repeated_words = (
> +                                        add => '',
> +                                        added => '',
> +                                        bad => '',
> +                                        be => '',
> +                                );
> +                                if ($first =~ /\b[0-9a-f]{2,}\b/) {
> +                                        next if (!exists($allow_repeated_words{lc($first)}));
> +                                }
> +
>  				if (WARN("REPEATED_WORD",
>  					 "Possible repeated word: '$first'\n" . $herecurr) &&
>  				    $fix) {
> 

Changes made in v4:
- Fix indentation
- Reduce the dictionary size for allowed words
- Apply changes over Dwaipayan's patch
(https://lore.kernel.org/linux-kernel-mentees/20201017162732.152351-1-dwaipayanray1@gmail.com/)

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
@ 2020-10-24  0:27   ` Aditya
  0 siblings, 0 replies; 8+ messages in thread
From: Aditya @ 2020-10-24  0:27 UTC (permalink / raw)
  To: joe; +Cc: linux-kernel-mentees, linux-kernel, dwaipayanray1

On 24/10/20 5:38 am, Aditya Srivastava wrote:
> Presence of hexadecimal address or symbol results in false warning
> message by checkpatch.pl.
> 
> For example, running checkpatch on commit b8ad540dd4e4 ("mptcp: fix
> memory leak in mptcp_subflow_create_socket()") results in warning:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'ff'
>     00 00 00 00 00 00 00 00 00 2f 30 0a 81 88 ff ff  ........./0.....
> 
> Similarly, the presence of list command output in commit results in
> an unnecessary warning.
> 
> For example, running checkpatch on commit 899e5ffbf246 ("perf record:
> Introduce --switch-output-event") gives:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'root'
>   dr-xr-x---. 12 root root    4096 Apr 27 17:46 ..
> 
> Here, it reports 'ff' and 'root to be repeated, but it is in fact part
> of some address or code, where it has to be repeated.
> 
> In these cases, the intent of the warning to find stylistic issues in
> commit messages is not met and the warning is just completely wrong in
> this case.
> 
> To avoid these warnings, add additional regex check for the
> directory permission pattern and avoid checking the line for this
> class of warning. Similarly, to avoid hex pattern, check if the word
> consists of hex symbols and skip this warning if it is not among the
> common english words formed using hex letters.
> 
> A quick evaluation on v5.6..v5.8 showed that this fix reduces
> REPEATED_WORD warnings from 2797 to 907.
> 
> A quick manual check found all cases are related to hex output or
> list command outputs in commit messages.
> 
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
>  scripts/checkpatch.pl | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7e505688257a..1d42d08d520b 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3049,7 +3049,9 @@ sub process {
>  		}
>  
>  # check for repeated words separated by a single space
> -		if ($rawline =~ /^\+/ || $in_commit_log) {
> +# avoid false positive from list command eg, '-rw-r--r-- 1 root root'
> +		if (($rawline =~ /^\+/ || $in_commit_log) &&
> +                    $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) {
>  			pos($rawline) = 1 if (!$in_commit_log);
>  			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
>  
> @@ -3074,6 +3076,17 @@ sub process {
>  				next if ($start_char =~ /^\S$/);
>  				next if (index(" \t.,;?!", $end_char) == -1);
>  
> +                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
> +                                my %allow_repeated_words = (
> +                                        add => '',
> +                                        added => '',
> +                                        bad => '',
> +                                        be => '',
> +                                );
> +                                if ($first =~ /\b[0-9a-f]{2,}\b/) {
> +                                        next if (!exists($allow_repeated_words{lc($first)}));
> +                                }
> +
>  				if (WARN("REPEATED_WORD",
>  					 "Possible repeated word: '$first'\n" . $herecurr) &&
>  				    $fix) {
> 

Changes made in v4:
- Fix indentation
- Reduce the dictionary size for allowed words
- Apply changes over Dwaipayan's patch
(https://lore.kernel.org/linux-kernel-mentees/20201017162732.152351-1-dwaipayanray1@gmail.com/)
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
  2020-10-24  0:08 ` [Linux-kernel-mentees] " Aditya Srivastava
@ 2020-10-24  1:37   ` Joe Perches
  -1 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-10-24  1:37 UTC (permalink / raw)
  To: Aditya Srivastava
  Cc: linux-kernel, linux-kernel-mentees, lukas.bulwahn, dwaipayanray1

On Sat, 2020-10-24 at 05:38 +0530, Aditya Srivastava wrote:
> Presence of hexadecimal address or symbol results in false warning
> message by checkpatch.pl.
> 
> For example, running checkpatch on commit b8ad540dd4e4 ("mptcp: fix
> memory leak in mptcp_subflow_create_socket()") results in warning:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'ff'
>     00 00 00 00 00 00 00 00 00 2f 30 0a 81 88 ff ff  ........./0.....
> 
> Similarly, the presence of list command output in commit results in
> an unnecessary warning.
> 
> For example, running checkpatch on commit 899e5ffbf246 ("perf record:
> Introduce --switch-output-event") gives:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'root'
>   dr-xr-x---. 12 root root    4096 Apr 27 17:46 ..
> 
> Here, it reports 'ff' and 'root to be repeated, but it is in fact part

'root'

> of some address or code, where it has to be repeated.
> 
> In these cases, the intent of the warning to find stylistic issues in
> commit messages is not met and the warning is just completely wrong in
> this case.
> 
> To avoid these warnings, add additional regex check for the

add an

> directory permission pattern and avoid checking the line for this
> class of warning. Similarly, to avoid hex pattern, check if the word
> consists of hex symbols and skip this warning if it is not among the
> common english words formed using hex letters.
> 
> A quick evaluation on v5.6..v5.8 showed that this fix reduces
> REPEATED_WORD warnings from 2797 to 907.

How many of these 907 remaining are still false positive?
 
> A quick manual check found all cases are related to hex output or
> list command outputs in commit messages.

You mean 1890 of the 2797 are now no longer reported and all 1890
were false positives yes?

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3049,7 +3049,9 @@ sub process {
>  		}
>  
> 
>  # check for repeated words separated by a single space
> -		if ($rawline =~ /^\+/ || $in_commit_log) {
> +# avoid false positive from list command eg, '-rw-r--r-- 1 root root'
> +		if (($rawline =~ /^\+/ || $in_commit_log) &&
> +                    $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) {

Use maximal tab indentation and spaces to align please.
2 tabs, 4 spaces

>  			pos($rawline) = 1 if (!$in_commit_log);
>  			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
>  
> 
> @@ -3074,6 +3076,17 @@ sub process {
>  				next if ($start_char =~ /^\S$/);
>  				next if (index(" \t.,;?!", $end_char) == -1);
>  
> 
> +                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
> +                                my %allow_repeated_words = (
> +                                        add => '',
> +                                        added => '',
> +                                        bad => '',
> +                                        be => '',
> +                                );

If perl caches this local hash declaration, fine,
but I think it better to use 'our %allow_repeated_words'
and move it so it's only declared using the file scope.

> +                                if ($first =~ /\b[0-9a-f]{2,}\b/) {

This regex matches only lower case so it wouldn't match "Add".

I think this regex would be clearer using
	/^[0-9a-f]+$/i or /^[A-Fa-f0-9]+$/



^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
@ 2020-10-24  1:37   ` Joe Perches
  0 siblings, 0 replies; 8+ messages in thread
From: Joe Perches @ 2020-10-24  1:37 UTC (permalink / raw)
  To: Aditya Srivastava; +Cc: linux-kernel-mentees, linux-kernel, dwaipayanray1

On Sat, 2020-10-24 at 05:38 +0530, Aditya Srivastava wrote:
> Presence of hexadecimal address or symbol results in false warning
> message by checkpatch.pl.
> 
> For example, running checkpatch on commit b8ad540dd4e4 ("mptcp: fix
> memory leak in mptcp_subflow_create_socket()") results in warning:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'ff'
>     00 00 00 00 00 00 00 00 00 2f 30 0a 81 88 ff ff  ........./0.....
> 
> Similarly, the presence of list command output in commit results in
> an unnecessary warning.
> 
> For example, running checkpatch on commit 899e5ffbf246 ("perf record:
> Introduce --switch-output-event") gives:
> 
> WARNING:REPEATED_WORD: Possible repeated word: 'root'
>   dr-xr-x---. 12 root root    4096 Apr 27 17:46 ..
> 
> Here, it reports 'ff' and 'root to be repeated, but it is in fact part

'root'

> of some address or code, where it has to be repeated.
> 
> In these cases, the intent of the warning to find stylistic issues in
> commit messages is not met and the warning is just completely wrong in
> this case.
> 
> To avoid these warnings, add additional regex check for the

add an

> directory permission pattern and avoid checking the line for this
> class of warning. Similarly, to avoid hex pattern, check if the word
> consists of hex symbols and skip this warning if it is not among the
> common english words formed using hex letters.
> 
> A quick evaluation on v5.6..v5.8 showed that this fix reduces
> REPEATED_WORD warnings from 2797 to 907.

How many of these 907 remaining are still false positive?
 
> A quick manual check found all cases are related to hex output or
> list command outputs in commit messages.

You mean 1890 of the 2797 are now no longer reported and all 1890
were false positives yes?

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3049,7 +3049,9 @@ sub process {
>  		}
>  
> 
>  # check for repeated words separated by a single space
> -		if ($rawline =~ /^\+/ || $in_commit_log) {
> +# avoid false positive from list command eg, '-rw-r--r-- 1 root root'
> +		if (($rawline =~ /^\+/ || $in_commit_log) &&
> +                    $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) {

Use maximal tab indentation and spaces to align please.
2 tabs, 4 spaces

>  			pos($rawline) = 1 if (!$in_commit_log);
>  			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
>  
> 
> @@ -3074,6 +3076,17 @@ sub process {
>  				next if ($start_char =~ /^\S$/);
>  				next if (index(" \t.,;?!", $end_char) == -1);
>  
> 
> +                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
> +                                my %allow_repeated_words = (
> +                                        add => '',
> +                                        added => '',
> +                                        bad => '',
> +                                        be => '',
> +                                );

If perl caches this local hash declaration, fine,
but I think it better to use 'our %allow_repeated_words'
and move it so it's only declared using the file scope.

> +                                if ($first =~ /\b[0-9a-f]{2,}\b/) {

This regex matches only lower case so it wouldn't match "Add".

I think this regex would be clearer using
	/^[0-9a-f]+$/i or /^[A-Fa-f0-9]+$/


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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
  2020-10-24  1:37   ` [Linux-kernel-mentees] " Joe Perches
@ 2020-10-24  9:28     ` Aditya
  -1 siblings, 0 replies; 8+ messages in thread
From: Aditya @ 2020-10-24  9:28 UTC (permalink / raw)
  To: Joe Perches
  Cc: linux-kernel, linux-kernel-mentees, lukas.bulwahn, dwaipayanray1

On 24/10/20 7:07 am, Joe Perches wrote:
> On Sat, 2020-10-24 at 05:38 +0530, Aditya Srivastava wrote: 
>> A quick evaluation on v5.6..v5.8 showed that this fix reduces
>> REPEATED_WORD warnings from 2797 to 907.
> 
> How many of these 907 remaining are still false positive?
>  
>> A quick manual check found all cases are related to hex output or
>> list command outputs in commit messages.
> 
> You mean 1890 of the 2797 are now no longer reported and all 1890
> were false positives yes?
> 

Yes. In v5.6..5.8, there were 2797 warnings for REPEATED_WORD, after
these changes, they are reduced to 907.
However, many among these 907 must have been fixed by Dwaipayan's
patch. I'll replace it with 1890 instead, for the better.

>>  			pos($rawline) = 1 if (!$in_commit_log);
>>  			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
>>  
>>
>> @@ -3074,6 +3076,17 @@ sub process {
>>  				next if ($start_char =~ /^\S$/);
>>  				next if (index(" \t.,;?!", $end_char) == -1);
>>  
>>
>> +                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
>> +                                my %allow_repeated_words = (
>> +                                        add => '',
>> +                                        added => '',
>> +                                        bad => '',
>> +                                        be => '',
>> +                                );
> 
> If perl caches this local hash declaration, fine,
> but I think it better to use 'our %allow_repeated_words'
> and move it so it's only declared using the file scope.
> 

I ran checkpatch over few commits, it was working fine. But I'll move
it to file scope, using 'our'. That should do as well.

>> +                                if ($first =~ /\b[0-9a-f]{2,}\b/) {
> 
> This regex matches only lower case so it wouldn't match "Add".
> 
> I think this regex would be clearer using
> 	/^[0-9a-f]+$/i or /^[A-Fa-f0-9]+$/
> 
> 

Missed it. Will do.

Thanks
Aditya

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning
@ 2020-10-24  9:28     ` Aditya
  0 siblings, 0 replies; 8+ messages in thread
From: Aditya @ 2020-10-24  9:28 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel-mentees, linux-kernel, dwaipayanray1

On 24/10/20 7:07 am, Joe Perches wrote:
> On Sat, 2020-10-24 at 05:38 +0530, Aditya Srivastava wrote: 
>> A quick evaluation on v5.6..v5.8 showed that this fix reduces
>> REPEATED_WORD warnings from 2797 to 907.
> 
> How many of these 907 remaining are still false positive?
>  
>> A quick manual check found all cases are related to hex output or
>> list command outputs in commit messages.
> 
> You mean 1890 of the 2797 are now no longer reported and all 1890
> were false positives yes?
> 

Yes. In v5.6..5.8, there were 2797 warnings for REPEATED_WORD, after
these changes, they are reduced to 907.
However, many among these 907 must have been fixed by Dwaipayan's
patch. I'll replace it with 1890 instead, for the better.

>>  			pos($rawline) = 1 if (!$in_commit_log);
>>  			while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) {
>>  
>>
>> @@ -3074,6 +3076,17 @@ sub process {
>>  				next if ($start_char =~ /^\S$/);
>>  				next if (index(" \t.,;?!", $end_char) == -1);
>>  
>>
>> +                                # avoid repeating hex occurrences like 'ff ff fe 09 ...'
>> +                                my %allow_repeated_words = (
>> +                                        add => '',
>> +                                        added => '',
>> +                                        bad => '',
>> +                                        be => '',
>> +                                );
> 
> If perl caches this local hash declaration, fine,
> but I think it better to use 'our %allow_repeated_words'
> and move it so it's only declared using the file scope.
> 

I ran checkpatch over few commits, it was working fine. But I'll move
it to file scope, using 'our'. That should do as well.

>> +                                if ($first =~ /\b[0-9a-f]{2,}\b/) {
> 
> This regex matches only lower case so it wouldn't match "Add".
> 
> I think this regex would be clearer using
> 	/^[0-9a-f]+$/i or /^[A-Fa-f0-9]+$/
> 
> 

Missed it. Will do.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-24  9:28 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-24  0:08 [PATCH v4] checkpatch: fix false positives in REPEATED_WORD warning Aditya Srivastava
2020-10-24  0:08 ` [Linux-kernel-mentees] " Aditya Srivastava
2020-10-24  0:27 ` Aditya
2020-10-24  0:27   ` [Linux-kernel-mentees] " Aditya
2020-10-24  1:37 ` Joe Perches
2020-10-24  1:37   ` [Linux-kernel-mentees] " Joe Perches
2020-10-24  9:28   ` Aditya
2020-10-24  9:28     ` [Linux-kernel-mentees] " Aditya

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.