* [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature
@ 2020-11-24 11:16 Aditya Srivastava
2020-11-24 11:48 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Aditya Srivastava @ 2020-11-24 11:16 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.
A large number of these warnings occur because of typo mistakes in
signoffs.
An evaluation over v4.13..v5.8 revealed that out of 539 warnings due to
Non-standard signatures, 85 are due to typo mistakes.
Eg. running checkpatch on commit da785a87787c ("ARM: bcm2835: Fix
integer overflow in rpi_firmware_print_firmware_revision()") reports
this warning:
WARNING:Non-standard signature: Revieved-by:
Revieved-by: Petr Mladek <pmladek@suse.com>
Here the signoff 'Reviewed-by' is misspelt.
Provide a fix by calculating levenshtein distance for the signoff over
all the standard signatures and suggest a fix if the distance for any
signature is less than or equal to 2.
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
applied on my last patch and next-20201120
scripts/checkpatch.pl | 73 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 72 insertions(+), 1 deletion(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b018deecec1a..2198360eebbd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
Cc:
)};
+sub get_min {
+ my (@arr) = @_;
+ my $len = scalar @arr;
+ if((scalar @arr) < 1) {
+ # if underflow, return
+ return;
+ }
+ my $min = $arr[0];
+ for my $i (0 .. ($len-1)) {
+ if ($arr[$i] < $min) {
+ $min = $arr[$i];
+ }
+ }
+ return $min;
+}
+
+sub get_edit_distance {
+ my ($str1, $str2) = @_;
+ my $len1 = length($str1);
+ my $len2 = length($str2);
+ # two dimensional array storing minimum edit distance
+ my @distance;
+ for my $i (0 .. $len1) {
+ for my $j (0 .. $len2) {
+ if ($i == 0) {
+ $distance[$i][$j] = $j;
+ }
+ elsif ($j == 0) {
+ $distance[$i][$j] = $i;
+ }
+ elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+ $distance[$i][$j] = $distance[$i - 1][$j - 1];
+ }
+ else {
+ my $dist1 = $distance[$i][$j - 1]; #insert distance
+ my $dist2 = $distance[$i - 1][$j]; # remove
+ my $dist3 = $distance[$i - 1][$j - 1]; #replace
+ $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+ }
+ }
+ }
+ return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+ my ($sign_off) = @_;
+ $sign_off = lc($sign_off);
+ $sign_off =~ s/\-//g; # to match with formed hash
+ my @standard_signature_tags = (
+ 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+ 'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
+ );
+ # setting default values
+ my $standard_signature = 'signed-off-by';
+ my $min_edit_distance = 20;
+ my $edit_distance;
+ foreach (@standard_signature_tags) {
+ my $signature = $_;
+ $_ =~ s/\-//g;
+ $edit_distance = get_edit_distance($sign_off, $_);
+ if ($edit_distance < $min_edit_distance) {
+ $min_edit_distance = $edit_distance;
+ $standard_signature = $signature;
+ }
+ }
+ if($min_edit_distance<=2) {
+ return ucfirst($standard_signature);
+ }
+ return "";
+}
+
our %standard_signature_fix = (
"Requested-by:" => {
suggestion => "Suggested-by:",
@@ -2848,7 +2919,7 @@ sub process {
my $ucfirst_sign_off = ucfirst(lc($sign_off));
if ($sign_off !~ /$signature_tags/) {
- my $suggested_signature = "";
+ my $suggested_signature = get_standard_signature($sign_off);
my $rationale = "";
if (exists($standard_signature_fix{$sign_off})) {
$suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
--
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] 12+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-24 11:16 [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature Aditya Srivastava
@ 2020-11-24 11:48 ` Lukas Bulwahn
2020-11-24 15:32 ` [Linux-kernel-mentees] [PATCH v2] " Aditya Srivastava
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Bulwahn @ 2020-11-24 11:48 UTC (permalink / raw)
To: Aditya Srivastava; +Cc: linux-kernel-mentees
On Tue, Nov 24, 2020 at 12:17 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signoffs.
> An evaluation over v4.13..v5.8 revealed that out of 539 warnings due to
> Non-standard signatures, 85 are due to typo mistakes.
>
> Eg. running checkpatch on commit da785a87787c ("ARM: bcm2835: Fix
> integer overflow in rpi_firmware_print_firmware_revision()") reports
> this warning:
>
> WARNING:Non-standard signature: Revieved-by:
> Revieved-by: Petr Mladek <pmladek@suse.com>
>
> Here the signoff 'Reviewed-by' is misspelt.
>
Again, it is better to provide a summary of the overall evaluation
instead of a single example,
> Provide a fix by calculating levenshtein distance for the signoff over
it is a signature tag, not the signoff.
> all the standard signatures and suggest a fix if the distance for any
> signature is less than or equal to 2.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> applied on my last patch and next-20201120
>
Joe, might ack this one and reject the other; so you might need to rebase later.
For the discussion with Joe now that is fine, though.
Other than that, it looks good.
> scripts/checkpatch.pl | 73 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index b018deecec1a..2198360eebbd 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
> Cc:
> )};
>
> +sub get_min {
> + my (@arr) = @_;
> + my $len = scalar @arr;
> + if((scalar @arr) < 1) {
> + # if underflow, return
> + return;
> + }
> + my $min = $arr[0];
> + for my $i (0 .. ($len-1)) {
> + if ($arr[$i] < $min) {
> + $min = $arr[$i];
> + }
> + }
> + return $min;
> +}
> +
> +sub get_edit_distance {
> + my ($str1, $str2) = @_;
> + my $len1 = length($str1);
> + my $len2 = length($str2);
> + # two dimensional array storing minimum edit distance
> + my @distance;
> + for my $i (0 .. $len1) {
> + for my $j (0 .. $len2) {
> + if ($i == 0) {
> + $distance[$i][$j] = $j;
> + }
> + elsif ($j == 0) {
> + $distance[$i][$j] = $i;
> + }
> + elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
> + $distance[$i][$j] = $distance[$i - 1][$j - 1];
> + }
> + else {
> + my $dist1 = $distance[$i][$j - 1]; #insert distance
> + my $dist2 = $distance[$i - 1][$j]; # remove
> + my $dist3 = $distance[$i - 1][$j - 1]; #replace
> + $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
> + }
> + }
> + }
> + return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> + my ($sign_off) = @_;
> + $sign_off = lc($sign_off);
> + $sign_off =~ s/\-//g; # to match with formed hash
> + my @standard_signature_tags = (
> + 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
> + 'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
> + );
> + # setting default values
> + my $standard_signature = 'signed-off-by';
> + my $min_edit_distance = 20;
> + my $edit_distance;
> + foreach (@standard_signature_tags) {
> + my $signature = $_;
> + $_ =~ s/\-//g;
> + $edit_distance = get_edit_distance($sign_off, $_);
> + if ($edit_distance < $min_edit_distance) {
> + $min_edit_distance = $edit_distance;
> + $standard_signature = $signature;
> + }
> + }
> + if($min_edit_distance<=2) {
> + return ucfirst($standard_signature);
> + }
> + return "";
> +}
> +
> our %standard_signature_fix = (
> "Requested-by:" => {
> suggestion => "Suggested-by:",
> @@ -2848,7 +2919,7 @@ sub process {
> my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
> if ($sign_off !~ /$signature_tags/) {
> - my $suggested_signature = "";
> + my $suggested_signature = get_standard_signature($sign_off);
> my $rationale = "";
> if (exists($standard_signature_fix{$sign_off})) {
> $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
> --
> 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 [flat|nested] 12+ messages in thread* [Linux-kernel-mentees] [PATCH v2] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-24 11:48 ` Lukas Bulwahn
@ 2020-11-24 15:32 ` Aditya Srivastava
2020-11-25 6:57 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Aditya Srivastava @ 2020-11-24 15:32 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.
A large number of these warnings occur because of typo mistakes in
signoffs. An evaluation over v4.13..v5.8 revealed that out of 539
warnings due to Non-standard signatures, 85 are due to typo mistakes.
Provide a fix by calculating levenshtein distance for the signature tag
over all the standard signatures and suggest a fix with signature, whose
edit distance is less than or equal to 2.
According to evaluation over v4.13..v5.8, following are the incorrect
signature tags with typos and their corresponding suggested fixes
(according to edit distace approach):
1)Reviwed-by (count: 19) => Reviewed-by
2)Reviewd-by (count: 9) => Reviewed-by
3)Singed-off-by (count: 8) => Signed-off-by
4)Signed-of-by (count: 6) => Signed-off-by
5)Rewieved-by (count: 3) => Reviewed-by
6)Signed-off--by (count: 3) => Signed-off-by
7)Revieved-by (count: 3) => Reviewed-by
8)Reivewed-by (count: 2) => Reviewed-by
9)Signef-off-by (count: 2) => Signed-off-by
10)Test-by (count: 2) => Tested-by
11)Acked_by (count: 2) => Acked-by
12)Signed-off-by-by (count: 2) => Signed-off-by
13)Reported-by-by (count: 1) => Reported-by
14)Reporetd-by (count: 1) => Reported-by
15)Reviewed--by (count: 1) => Reviewed-by
16)Sugested-by (count: 1) => Suggested-by
17)Suggested--by (count: 1) => Suggested-by
18)Repoted-by (count: 1) => Reported-by
19)Rported-by (count: 1) => Reported-by
20)eigned-off-by (count: 1) => Signed-off-by
21)Reveiwed-by (count: 1) => Reviewed-by
22)igned-off-by (count: 1) => Signed-off-by
23)Tested-by-by (count: 1) => Tested-by
24)Sugessted-by (count: 1) => Suggested-by
25)Rewiewed-by (count: 1) => Reviewed-by
26)Teste-by (count: 1) => Tested-by
27)Signee-off-by (count: 1) => Signed-off-by
28)-By (count: 1) => To
29)Signen-off-by (count: 1) => Signed-off-by
30)eported-by (count: 1) => Reported-by
31)Reviewedy-by (count: 1) => Signed-off-by
32)Siganed-off-by (count: 1) => Signed-off-by
33)Ackedy-by (count: 1) => Acked-by
34)Review-by (count: 1) => Reviewed-by
35)Tweeted-by (count: 1) => Tested-by
36)Ack-by (count: 1) => Acked-by
37)Reorted-by (count: 1) => Reported-by
38)Suggsted-by (count: 1) => Suggested-by
39)Reviwed-By (count: 1) => Reviewed-by
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
patch applies on next-20201120
scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..775a49a06179 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
Cc:
)};
+sub get_min {
+ my (@arr) = @_;
+ my $len = scalar @arr;
+ if((scalar @arr) < 1) {
+ # if underflow, return
+ return;
+ }
+ my $min = $arr[0];
+ for my $i (0 .. ($len-1)) {
+ if ($arr[$i] < $min) {
+ $min = $arr[$i];
+ }
+ }
+ return $min;
+}
+
+sub get_edit_distance {
+ my ($str1, $str2) = @_;
+ my $len1 = length($str1);
+ my $len2 = length($str2);
+ # two dimensional array storing minimum edit distance
+ my @distance;
+ for my $i (0 .. $len1) {
+ for my $j (0 .. $len2) {
+ if ($i == 0) {
+ $distance[$i][$j] = $j;
+ }
+ elsif ($j == 0) {
+ $distance[$i][$j] = $i;
+ }
+ elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+ $distance[$i][$j] = $distance[$i - 1][$j - 1];
+ }
+ else {
+ my $dist1 = $distance[$i][$j - 1]; #insert distance
+ my $dist2 = $distance[$i - 1][$j]; # remove
+ my $dist3 = $distance[$i - 1][$j - 1]; #replace
+ $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+ }
+ }
+ }
+ return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+ my ($sign_off) = @_;
+ $sign_off = lc($sign_off);
+ $sign_off =~ s/\-//g; # to match with formed hash
+ my @standard_signature_tags = (
+ 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+ 'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
+ );
+ # setting default values
+ my $standard_signature = 'signed-off-by';
+ my $min_edit_distance = 20;
+ my $edit_distance;
+ foreach (@standard_signature_tags) {
+ my $signature = $_;
+ $_ =~ s/\-//g;
+ $edit_distance = get_edit_distance($sign_off, $_);
+ if ($edit_distance < $min_edit_distance) {
+ $min_edit_distance = $edit_distance;
+ $standard_signature = $signature;
+ }
+ }
+ if($min_edit_distance<=2) {
+ return ucfirst($standard_signature);
+ }
+ return "";
+}
+
our @typeListMisordered = (
qr{char\s+(?:un)?signed},
qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2844,18 @@ sub process {
my $ucfirst_sign_off = ucfirst(lc($sign_off));
if ($sign_off !~ /$signature_tags/) {
- WARN("BAD_SIGN_OFF",
- "Non-standard signature: $sign_off\n" . $herecurr);
+ my $suggested_signature = get_standard_signature($sign_off);
+ if ($suggested_signature eq "") {
+ WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off\n" . $herecurr);
+ }
+ else {
+ if (WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+ }
+ }
}
if (defined $space_before && $space_before ne "") {
if (WARN("BAD_SIGN_OFF",
--
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] 12+ messages in thread* Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-24 15:32 ` [Linux-kernel-mentees] [PATCH v2] " Aditya Srivastava
@ 2020-11-25 6:57 ` Lukas Bulwahn
2020-11-25 11:25 ` [Linux-kernel-mentees] [PATCH v3] " Aditya Srivastava
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Bulwahn @ 2020-11-25 6:57 UTC (permalink / raw)
To: Aditya Srivastava; +Cc: linux-kernel-mentees
On Tue, Nov 24, 2020 at 4:32 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signoffs. An evaluation over v4.13..v5.8 revealed that out of 539
> warnings due to Non-standard signatures, 85 are due to typo mistakes.
>
> Provide a fix by calculating levenshtein distance for the signature tag
> over all the standard signatures and suggest a fix with signature, whose
> edit distance is less than or equal to 2.
>
> According to evaluation over v4.13..v5.8, following are the incorrect
> signature tags with typos and their corresponding suggested fixes
> (according to edit distace approach):
>
> 1)Reviwed-by (count: 19) => Reviewed-by
>
> 2)Reviewd-by (count: 9) => Reviewed-by
>
> 3)Singed-off-by (count: 8) => Signed-off-by
>
> 4)Signed-of-by (count: 6) => Signed-off-by
>
> 5)Rewieved-by (count: 3) => Reviewed-by
>
> 6)Signed-off--by (count: 3) => Signed-off-by
>
> 7)Revieved-by (count: 3) => Reviewed-by
>
> 8)Reivewed-by (count: 2) => Reviewed-by
>
> 9)Signef-off-by (count: 2) => Signed-off-by
>
> 10)Test-by (count: 2) => Tested-by
>
> 11)Acked_by (count: 2) => Acked-by
>
> 12)Signed-off-by-by (count: 2) => Signed-off-by
>
> 13)Reported-by-by (count: 1) => Reported-by
>
> 14)Reporetd-by (count: 1) => Reported-by
>
> 15)Reviewed--by (count: 1) => Reviewed-by
>
> 16)Sugested-by (count: 1) => Suggested-by
>
> 17)Suggested--by (count: 1) => Suggested-by
>
> 18)Repoted-by (count: 1) => Reported-by
>
> 19)Rported-by (count: 1) => Reported-by
>
> 20)eigned-off-by (count: 1) => Signed-off-by
>
> 21)Reveiwed-by (count: 1) => Reviewed-by
>
> 22)igned-off-by (count: 1) => Signed-off-by
>
> 23)Tested-by-by (count: 1) => Tested-by
>
> 24)Sugessted-by (count: 1) => Suggested-by
>
> 25)Rewiewed-by (count: 1) => Reviewed-by
>
> 26)Teste-by (count: 1) => Tested-by
>
> 27)Signee-off-by (count: 1) => Signed-off-by
>
> 28)-By (count: 1) => To
>
> 29)Signen-off-by (count: 1) => Signed-off-by
>
> 30)eported-by (count: 1) => Reported-by
>
> 31)Reviewedy-by (count: 1) => Signed-off-by
>
> 32)Siganed-off-by (count: 1) => Signed-off-by
>
> 33)Ackedy-by (count: 1) => Acked-by
>
> 34)Review-by (count: 1) => Reviewed-by
>
> 35)Tweeted-by (count: 1) => Tested-by
>
> 36)Ack-by (count: 1) => Acked-by
>
> 37)Reorted-by (count: 1) => Reported-by
>
> 38)Suggsted-by (count: 1) => Suggested-by
>
> 39)Reviwed-By (count: 1) => Reviewed-by
>
How about summarizing this?
Which are good corrections, which are "bad corrections"; what is the
total count of corrections, how many are good, and which are bad?
How many typo variants were corrected to each valid tag?
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
> patch applies on next-20201120
>
> scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..775a49a06179 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
> Cc:
> )};
>
> +sub get_min {
> + my (@arr) = @_;
> + my $len = scalar @arr;
> + if((scalar @arr) < 1) {
> + # if underflow, return
> + return;
> + }
> + my $min = $arr[0];
> + for my $i (0 .. ($len-1)) {
> + if ($arr[$i] < $min) {
> + $min = $arr[$i];
> + }
> + }
> + return $min;
> +}
> +
> +sub get_edit_distance {
> + my ($str1, $str2) = @_;
> + my $len1 = length($str1);
> + my $len2 = length($str2);
> + # two dimensional array storing minimum edit distance
> + my @distance;
> + for my $i (0 .. $len1) {
> + for my $j (0 .. $len2) {
> + if ($i == 0) {
> + $distance[$i][$j] = $j;
> + }
> + elsif ($j == 0) {
> + $distance[$i][$j] = $i;
> + }
> + elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
> + $distance[$i][$j] = $distance[$i - 1][$j - 1];
> + }
> + else {
> + my $dist1 = $distance[$i][$j - 1]; #insert distance
> + my $dist2 = $distance[$i - 1][$j]; # remove
> + my $dist3 = $distance[$i - 1][$j - 1]; #replace
> + $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
> + }
> + }
> + }
> + return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> + my ($sign_off) = @_;
> + $sign_off = lc($sign_off);
> + $sign_off =~ s/\-//g; # to match with formed hash
> + my @standard_signature_tags = (
> + 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
> + 'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
> + );
> + # setting default values
> + my $standard_signature = 'signed-off-by';
> + my $min_edit_distance = 20;
> + my $edit_distance;
> + foreach (@standard_signature_tags) {
> + my $signature = $_;
> + $_ =~ s/\-//g;
> + $edit_distance = get_edit_distance($sign_off, $_);
> + if ($edit_distance < $min_edit_distance) {
> + $min_edit_distance = $edit_distance;
> + $standard_signature = $signature;
> + }
> + }
> + if($min_edit_distance<=2) {
> + return ucfirst($standard_signature);
> + }
> + return "";
> +}
> +
> our @typeListMisordered = (
> qr{char\s+(?:un)?signed},
> qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2844,18 @@ sub process {
> my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
> if ($sign_off !~ /$signature_tags/) {
> - WARN("BAD_SIGN_OFF",
> - "Non-standard signature: $sign_off\n" . $herecurr);
> + my $suggested_signature = get_standard_signature($sign_off);
> + if ($suggested_signature eq "") {
> + WARN("BAD_SIGN_OFF",
> + "Non-standard signature: $sign_off\n" . $herecurr);
> + }
> + else {
> + if (WARN("BAD_SIGN_OFF",
> + "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> + }
> + }
> }
> if (defined $space_before && $space_before ne "") {
> if (WARN("BAD_SIGN_OFF",
> --
> 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 [flat|nested] 12+ messages in thread* [Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-25 6:57 ` Lukas Bulwahn
@ 2020-11-25 11:25 ` Aditya Srivastava
2020-11-25 12:26 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Aditya Srivastava @ 2020-11-25 11:25 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.
A large number of these warnings occur because of typo mistakes in
signoffs. An evaluation over v4.13..v5.8 revealed that out of 539
warnings due to Non-standard signatures, 87 are due to typo mistakes.
Following are the standard signature tags which are often incorrectly
used with their counts(over v4.13..v5.8):
1) Reviewed-by => 42
2) Signed-off-by => 25
3) Reported-by => 6
4) Acked-by => 4
5) Tested-by => 4
6) Suggested-by => 4
Provide a fix by calculating levenshtein distance for the signature tag
with all the standard signatures and suggest a fix with signature, whose
edit distance is less than or equal to 2 with the misspelt signature.
Out of the 87 misspelt signatures fixed with this approach, 85 were
found to be good corrections and 2 were bad corrections.
The signature tags which are good corrections using this approach are:
1)Reviwed-by (count: 19) => Reviewed-by
2)Reviewd-by (count: 9) => Reviewed-by
3)Singed-off-by (count: 8) => Signed-off-by
4)Signed-of-by (count: 6) => Signed-off-by
5)Rewieved-by (count: 3) => Reviewed-by
6)Signed-off--by (count: 3) => Signed-off-by
7)Revieved-by (count: 3) => Reviewed-by
8)Reivewed-by (count: 2) => Reviewed-by
9)Signef-off-by (count: 2) => Signed-off-by
10)Test-by (count: 2) => Tested-by
11)Acked_by (count: 2) => Acked-by
12)Signed-off-by-by (count: 2) => Signed-off-by
13)Reported-by-by (count: 1) => Reported-by
14)Reporetd-by (count: 1) => Reported-by
15)Reviewed--by (count: 1) => Reviewed-by
16)Sugested-by (count: 1) => Suggested-by
17)Suggested--by (count: 1) => Suggested-by
18)Repoted-by (count: 1) => Reported-by
19)Rported-by (count: 1) => Reported-by
20)eigned-off-by (count: 1) => Signed-off-by
21)Reveiwed-by (count: 1) => Reviewed-by
22)igned-off-by (count: 1) => Signed-off-by
23)Tested-by-by (count: 1) => Tested-by
24)Sugessted-by (count: 1) => Suggested-by
25)Rewiewed-by (count: 1) => Reviewed-by
26)Teste-by (count: 1) => Tested-by
27)Signee-off-by (count: 1) => Signed-off-by
28)Signen-off-by (count: 1) => Signed-off-by
29)Reviwed-By (count: 1) => Reviewed-by
30)eported-by (count: 1) => Reported-by
31)Reviewedy-by (count: 1) => Reviewed-by
32)Siganed-off-by (count: 1) => Signed-off-by
33)Ackedy-by (count: 1) => Acked-by
34)Review-by (count: 1) => Reviewed-by
35)Suggsted-by (count: 1) => Suggested-by
36)Ack-by (count: 1) => Acked-by
37)Reorted-by (count: 1) => Reported-by
Following were found to be bad corrections:
1)-By (count: 1) => To
2)Tweeted-by (count: 1) => Tested-by
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
changes in v3: summarize commit message
scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..775a49a06179 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
Cc:
)};
+sub get_min {
+ my (@arr) = @_;
+ my $len = scalar @arr;
+ if((scalar @arr) < 1) {
+ # if underflow, return
+ return;
+ }
+ my $min = $arr[0];
+ for my $i (0 .. ($len-1)) {
+ if ($arr[$i] < $min) {
+ $min = $arr[$i];
+ }
+ }
+ return $min;
+}
+
+sub get_edit_distance {
+ my ($str1, $str2) = @_;
+ my $len1 = length($str1);
+ my $len2 = length($str2);
+ # two dimensional array storing minimum edit distance
+ my @distance;
+ for my $i (0 .. $len1) {
+ for my $j (0 .. $len2) {
+ if ($i == 0) {
+ $distance[$i][$j] = $j;
+ }
+ elsif ($j == 0) {
+ $distance[$i][$j] = $i;
+ }
+ elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+ $distance[$i][$j] = $distance[$i - 1][$j - 1];
+ }
+ else {
+ my $dist1 = $distance[$i][$j - 1]; #insert distance
+ my $dist2 = $distance[$i - 1][$j]; # remove
+ my $dist3 = $distance[$i - 1][$j - 1]; #replace
+ $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+ }
+ }
+ }
+ return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+ my ($sign_off) = @_;
+ $sign_off = lc($sign_off);
+ $sign_off =~ s/\-//g; # to match with formed hash
+ my @standard_signature_tags = (
+ 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+ 'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
+ );
+ # setting default values
+ my $standard_signature = 'signed-off-by';
+ my $min_edit_distance = 20;
+ my $edit_distance;
+ foreach (@standard_signature_tags) {
+ my $signature = $_;
+ $_ =~ s/\-//g;
+ $edit_distance = get_edit_distance($sign_off, $_);
+ if ($edit_distance < $min_edit_distance) {
+ $min_edit_distance = $edit_distance;
+ $standard_signature = $signature;
+ }
+ }
+ if($min_edit_distance<=2) {
+ return ucfirst($standard_signature);
+ }
+ return "";
+}
+
our @typeListMisordered = (
qr{char\s+(?:un)?signed},
qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2844,18 @@ sub process {
my $ucfirst_sign_off = ucfirst(lc($sign_off));
if ($sign_off !~ /$signature_tags/) {
- WARN("BAD_SIGN_OFF",
- "Non-standard signature: $sign_off\n" . $herecurr);
+ my $suggested_signature = get_standard_signature($sign_off);
+ if ($suggested_signature eq "") {
+ WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off\n" . $herecurr);
+ }
+ else {
+ if (WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+ }
+ }
}
if (defined $space_before && $space_before ne "") {
if (WARN("BAD_SIGN_OFF",
--
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] 12+ messages in thread* Re: [Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-25 11:25 ` [Linux-kernel-mentees] [PATCH v3] " Aditya Srivastava
@ 2020-11-25 12:26 ` Lukas Bulwahn
2020-11-28 9:52 ` [Linux-kernel-mentees] [PATCH v4] checkpatch: add fix and improve warning msg for non-standard signature Aditya Srivastava
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Bulwahn @ 2020-11-25 12:26 UTC (permalink / raw)
To: Aditya Srivastava; +Cc: linux-kernel-mentees
On Wed, Nov 25, 2020 at 12:25 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signoffs. An evaluation over v4.13..v5.8 revealed that out of 539
It is not a sign-off, it is a signature tag.
> warnings due to Non-standard signatures, 87 are due to typo mistakes.
>
why do you write "non-standard" capitalized?
> Following are the standard signature tags which are often incorrectly
> used with their counts(over v4.13..v5.8):
>
This sentence above is not clear.
> 1) Reviewed-by => 42
> 2) Signed-off-by => 25
> 3) Reported-by => 6
> 4) Acked-by => 4
> 5) Tested-by => 4
> 6) Suggested-by => 4
>
You can probably drop the 1) to 6); simply indent it; I think "tag:
count" or "tag count" (where the spacing is aligned) is better
than "tag => count"
> Provide a fix by calculating levenshtein distance for the signature tag
> with all the standard signatures and suggest a fix with signature, whose
> edit distance is less than or equal to 2 with the misspelt signature.
>
s/misspelt/misspelled/
> Out of the 87 misspelt signatures fixed with this approach, 85 were
> found to be good corrections and 2 were bad corrections.
>
> The signature tags which are good corrections using this approach are:
>
> 1)Reviwed-by (count: 19) => Reviewed-by
> 2)Reviewd-by (count: 9) => Reviewed-by
> 3)Singed-off-by (count: 8) => Signed-off-by
> 4)Signed-of-by (count: 6) => Signed-off-by
> 5)Rewieved-by (count: 3) => Reviewed-by
> 6)Signed-off--by (count: 3) => Signed-off-by
> 7)Revieved-by (count: 3) => Reviewed-by
> 8)Reivewed-by (count: 2) => Reviewed-by
> 9)Signef-off-by (count: 2) => Signed-off-by
> 10)Test-by (count: 2) => Tested-by
> 11)Acked_by (count: 2) => Acked-by
> 12)Signed-off-by-by (count: 2) => Signed-off-by
> 13)Reported-by-by (count: 1) => Reported-by
> 14)Reporetd-by (count: 1) => Reported-by
> 15)Reviewed--by (count: 1) => Reviewed-by
> 16)Sugested-by (count: 1) => Suggested-by
> 17)Suggested--by (count: 1) => Suggested-by
> 18)Repoted-by (count: 1) => Reported-by
> 19)Rported-by (count: 1) => Reported-by
> 20)eigned-off-by (count: 1) => Signed-off-by
> 21)Reveiwed-by (count: 1) => Reviewed-by
> 22)igned-off-by (count: 1) => Signed-off-by
> 23)Tested-by-by (count: 1) => Tested-by
> 24)Sugessted-by (count: 1) => Suggested-by
> 25)Rewiewed-by (count: 1) => Reviewed-by
> 26)Teste-by (count: 1) => Tested-by
> 27)Signee-off-by (count: 1) => Signed-off-by
> 28)Signen-off-by (count: 1) => Signed-off-by
> 29)Reviwed-By (count: 1) => Reviewed-by
> 30)eported-by (count: 1) => Reported-by
> 31)Reviewedy-by (count: 1) => Reviewed-by
> 32)Siganed-off-by (count: 1) => Signed-off-by
> 33)Ackedy-by (count: 1) => Acked-by
> 34)Review-by (count: 1) => Reviewed-by
> 35)Suggsted-by (count: 1) => Suggested-by
> 36)Ack-by (count: 1) => Acked-by
> 37)Reorted-by (count: 1) => Reported-by
>
I do not think the enumeration of all those typos is interesting for
the commit message. Maybe Joe will ask, but anyone else can imagine
how such typos look like.
If you think they should be kept, I would just sort it this way:
Typo variants of the tags were:
Signed-off-by:
then list all variants
Acked-by:
then list all variants
Reported-by:
then list all variants
etc.
> Following were found to be bad corrections:
>
> 1)-By (count: 1) => To
How about not correcting to To: (It is simply too short).
The explanation is getting better, give it another try. Then we might
be in a good state for Joe and lkml.
Lukas
> 2)Tweeted-by (count: 1) => Tested-by
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
>
> changes in v3: summarize commit message
>
> scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..775a49a06179 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
> Cc:
> )};
>
> +sub get_min {
> + my (@arr) = @_;
> + my $len = scalar @arr;
> + if((scalar @arr) < 1) {
> + # if underflow, return
> + return;
> + }
> + my $min = $arr[0];
> + for my $i (0 .. ($len-1)) {
> + if ($arr[$i] < $min) {
> + $min = $arr[$i];
> + }
> + }
> + return $min;
> +}
> +
> +sub get_edit_distance {
> + my ($str1, $str2) = @_;
> + my $len1 = length($str1);
> + my $len2 = length($str2);
> + # two dimensional array storing minimum edit distance
> + my @distance;
> + for my $i (0 .. $len1) {
> + for my $j (0 .. $len2) {
> + if ($i == 0) {
> + $distance[$i][$j] = $j;
> + }
> + elsif ($j == 0) {
> + $distance[$i][$j] = $i;
> + }
> + elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
> + $distance[$i][$j] = $distance[$i - 1][$j - 1];
> + }
> + else {
> + my $dist1 = $distance[$i][$j - 1]; #insert distance
> + my $dist2 = $distance[$i - 1][$j]; # remove
> + my $dist3 = $distance[$i - 1][$j - 1]; #replace
> + $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
> + }
> + }
> + }
> + return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> + my ($sign_off) = @_;
> + $sign_off = lc($sign_off);
> + $sign_off =~ s/\-//g; # to match with formed hash
> + my @standard_signature_tags = (
> + 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
> + 'reviewed-by:', 'reported-by:', 'suggested-by:', 'to:', 'cc:'
> + );
> + # setting default values
> + my $standard_signature = 'signed-off-by';
> + my $min_edit_distance = 20;
> + my $edit_distance;
> + foreach (@standard_signature_tags) {
> + my $signature = $_;
> + $_ =~ s/\-//g;
> + $edit_distance = get_edit_distance($sign_off, $_);
> + if ($edit_distance < $min_edit_distance) {
> + $min_edit_distance = $edit_distance;
> + $standard_signature = $signature;
> + }
> + }
> + if($min_edit_distance<=2) {
> + return ucfirst($standard_signature);
> + }
> + return "";
> +}
> +
> our @typeListMisordered = (
> qr{char\s+(?:un)?signed},
> qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2844,18 @@ sub process {
> my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
> if ($sign_off !~ /$signature_tags/) {
> - WARN("BAD_SIGN_OFF",
> - "Non-standard signature: $sign_off\n" . $herecurr);
> + my $suggested_signature = get_standard_signature($sign_off);
> + if ($suggested_signature eq "") {
> + WARN("BAD_SIGN_OFF",
> + "Non-standard signature: $sign_off\n" . $herecurr);
> + }
> + else {
> + if (WARN("BAD_SIGN_OFF",
> + "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> + }
> + }
> }
> if (defined $space_before && $space_before ne "") {
> if (WARN("BAD_SIGN_OFF",
> --
> 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 [flat|nested] 12+ messages in thread* [Linux-kernel-mentees] [PATCH v4] checkpatch: add fix and improve warning msg for non-standard signature
2020-11-25 12:26 ` Lukas Bulwahn
@ 2020-11-28 9:52 ` Aditya Srivastava
2020-11-28 13:00 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Aditya Srivastava @ 2020-11-28 9:52 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
styles.
A large number of these warnings occur because of typo mistakes in
signature tags. An evaluation over v4.13..v5.8 showed that out of 539
warnings due to non-standard signatures, 87 are due to typo mistakes.
Following are the standard signature tags which are often incorrectly
used, along with their individual counts of incorrect use (over
v4.13..v5.8):
Reviewed-by: 42
Signed-off-by: 25
Reported-by: 6
Acked-by: 4
Tested-by: 4
Suggested-by: 4
Provide a fix by calculating levenshtein distance for the signature tag
with all the standard signatures and suggest a fix with a signature, whose
edit distance is less than or equal to 2 with the misspelled signature.
Out of the 86 mispelled signatures fixed with this approach, 85 were
found to be good corrections and 1 was bad correction.
Following was found to be a bad correction:
Tweeted-by (count: 1) => Tested-by
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: modify commit message: replace specific example with overall evaluation, minor changes
changes in v3: summarize commit message
changes in v4: improve commit message; remove signature suggestions of small length (ie 'cc' and 'to')
scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 83 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..2b1afd763d8d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
Cc:
)};
+sub get_min {
+ my (@arr) = @_;
+ my $len = scalar @arr;
+ if((scalar @arr) < 1) {
+ # if underflow, return
+ return;
+ }
+ my $min = $arr[0];
+ for my $i (0 .. ($len-1)) {
+ if ($arr[$i] < $min) {
+ $min = $arr[$i];
+ }
+ }
+ return $min;
+}
+
+sub get_edit_distance {
+ my ($str1, $str2) = @_;
+ my $len1 = length($str1);
+ my $len2 = length($str2);
+ # two dimensional array storing minimum edit distance
+ my @distance;
+ for my $i (0 .. $len1) {
+ for my $j (0 .. $len2) {
+ if ($i == 0) {
+ $distance[$i][$j] = $j;
+ }
+ elsif ($j == 0) {
+ $distance[$i][$j] = $i;
+ }
+ elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) {
+ $distance[$i][$j] = $distance[$i - 1][$j - 1];
+ }
+ else {
+ my $dist1 = $distance[$i][$j - 1]; #insert distance
+ my $dist2 = $distance[$i - 1][$j]; # remove
+ my $dist3 = $distance[$i - 1][$j - 1]; #replace
+ $distance[$i][$j] = 1 + get_min($dist1, $dist2, $dist3);
+ }
+ }
+ }
+ return $distance[$len1][$len2];
+}
+
+sub get_standard_signature {
+ my ($sign_off) = @_;
+ $sign_off = lc($sign_off);
+ $sign_off =~ s/\-//g; # to match with formed hash
+ my @standard_signature_tags = (
+ 'signed-off-by:', 'co-developed-by:', 'acked-by:', 'tested-by:',
+ 'reviewed-by:', 'reported-by:', 'suggested-by:'
+ );
+ # setting default values
+ my $standard_signature = 'signed-off-by';
+ my $min_edit_distance = 20;
+ my $edit_distance;
+ foreach (@standard_signature_tags) {
+ my $signature = $_;
+ $_ =~ s/\-//g;
+ $edit_distance = get_edit_distance($sign_off, $_);
+ if ($edit_distance < $min_edit_distance) {
+ $min_edit_distance = $edit_distance;
+ $standard_signature = $signature;
+ }
+ }
+ if($min_edit_distance<=2) {
+ return ucfirst($standard_signature);
+ }
+ return "";
+}
+
our @typeListMisordered = (
qr{char\s+(?:un)?signed},
qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2844,18 @@ sub process {
my $ucfirst_sign_off = ucfirst(lc($sign_off));
if ($sign_off !~ /$signature_tags/) {
- WARN("BAD_SIGN_OFF",
- "Non-standard signature: $sign_off\n" . $herecurr);
+ my $suggested_signature = get_standard_signature($sign_off);
+ if ($suggested_signature eq "") {
+ WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off\n" . $herecurr);
+ }
+ else {
+ if (WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+ }
+ }
}
if (defined $space_before && $space_before ne "") {
if (WARN("BAD_SIGN_OFF",
--
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] 12+ messages in thread* Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: add fix and improve warning msg for non-standard signature
2020-11-28 9:52 ` [Linux-kernel-mentees] [PATCH v4] checkpatch: add fix and improve warning msg for non-standard signature Aditya Srivastava
@ 2020-11-28 13:00 ` Lukas Bulwahn
0 siblings, 0 replies; 12+ messages in thread
From: Lukas Bulwahn @ 2020-11-28 13:00 UTC (permalink / raw)
To: Aditya Srivastava; +Cc: linux-kernel-mentees
[-- Attachment #1.1: Type: text/plain, Size: 6014 bytes --]
Aditya Srivastava <yashsri421@gmail.com> schrieb am Sa., 28. Nov. 2020 um
10:52:
> Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> A large number of these warnings occur because of typo mistakes in
> signature tags. An evaluation over v4.13..v5.8 showed that out of 539
> warnings due to non-standard signatures, 87 are due to typo mistakes.
>
> Following are the standard signature tags which are often incorrectly
> used, along with their individual counts of incorrect use (over
> v4.13..v5.8):
>
> Reviewed-by: 42
> Signed-off-by: 25
> Reported-by: 6
> Acked-by: 4
> Tested-by: 4
> Suggested-by: 4
>
> Provide a fix by calculating levenshtein distance for the signature tag
> with all the standard signatures and suggest a fix with a signature, whose
> edit distance is less than or equal to 2 with the misspelled signature.
>
> Out of the 86 mispelled signatures fixed with this approach, 85 were
> found to be good corrections and 1 was bad correction.
>
> Following was found to be a bad correction:
> Tweeted-by (count: 1) => Tested-by
>
Looks Good. Let us send it to Joe.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: modify commit message: replace specific example with
> overall evaluation, minor changes
>
> changes in v3: summarize commit message
>
> changes in v4: improve commit message; remove signature suggestions of
> small length (ie 'cc' and 'to')
>
> scripts/checkpatch.pl | 85 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..2b1afd763d8d 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,77 @@ our $signature_tags = qr{(?xi:
> Cc:
> )};
>
> +sub get_min {
> + my (@arr) = @_;
> + my $len = scalar @arr;
> + if((scalar @arr) < 1) {
> + # if underflow, return
> + return;
> + }
> + my $min = $arr[0];
> + for my $i (0 .. ($len-1)) {
> + if ($arr[$i] < $min) {
> + $min = $arr[$i];
> + }
> + }
> + return $min;
> +}
> +
> +sub get_edit_distance {
> + my ($str1, $str2) = @_;
> + my $len1 = length($str1);
> + my $len2 = length($str2);
> + # two dimensional array storing minimum edit distance
> + my @distance;
> + for my $i (0 .. $len1) {
> + for my $j (0 .. $len2) {
> + if ($i == 0) {
> + $distance[$i][$j] = $j;
> + }
> + elsif ($j == 0) {
> + $distance[$i][$j] = $i;
> + }
> + elsif (substr($str1, $i-1, 1) eq substr($str2,
> $j-1, 1)) {
> + $distance[$i][$j] = $distance[$i - 1][$j -
> 1];
> + }
> + else {
> + my $dist1 = $distance[$i][$j - 1]; #insert
> distance
> + my $dist2 = $distance[$i - 1][$j]; # remove
> + my $dist3 = $distance[$i - 1][$j - 1];
> #replace
> + $distance[$i][$j] = 1 + get_min($dist1,
> $dist2, $dist3);
> + }
> + }
> + }
> + return $distance[$len1][$len2];
> +}
> +
> +sub get_standard_signature {
> + my ($sign_off) = @_;
> + $sign_off = lc($sign_off);
> + $sign_off =~ s/\-//g; # to match with formed hash
> + my @standard_signature_tags = (
> + 'signed-off-by:', 'co-developed-by:', 'acked-by:',
> 'tested-by:',
> + 'reviewed-by:', 'reported-by:', 'suggested-by:'
> + );
> + # setting default values
> + my $standard_signature = 'signed-off-by';
> + my $min_edit_distance = 20;
> + my $edit_distance;
> + foreach (@standard_signature_tags) {
> + my $signature = $_;
> + $_ =~ s/\-//g;
> + $edit_distance = get_edit_distance($sign_off, $_);
> + if ($edit_distance < $min_edit_distance) {
> + $min_edit_distance = $edit_distance;
> + $standard_signature = $signature;
> + }
> + }
> + if($min_edit_distance<=2) {
> + return ucfirst($standard_signature);
> + }
> + return "";
> +}
> +
> our @typeListMisordered = (
> qr{char\s+(?:un)?signed},
> qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2844,18 @@ sub process {
> my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
> if ($sign_off !~ /$signature_tags/) {
> - WARN("BAD_SIGN_OFF",
> - "Non-standard signature: $sign_off\n"
> . $herecurr);
> + my $suggested_signature =
> get_standard_signature($sign_off);
> + if ($suggested_signature eq "") {
> + WARN("BAD_SIGN_OFF",
> + "Non-standard signature:
> $sign_off\n" . $herecurr);
> + }
> + else {
> + if (WARN("BAD_SIGN_OFF",
> + "Non-standard signature:
> $sign_off. Please use '$suggested_signature' instead\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~
> s/$sign_off/$suggested_signature/;
> + }
> + }
> }
> if (defined $space_before && $space_before ne "") {
> if (WARN("BAD_SIGN_OFF",
> --
> 2.17.1
>
>
[-- Attachment #1.2: Type: text/html, Size: 8294 bytes --]
[-- Attachment #2: Type: text/plain, Size: 201 bytes --]
_______________________________________________
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] 12+ messages in thread
* Re: [Linux-kernel-mentees] Fix for BAD_SIGN_OFF: non-standard signature
@ 2020-11-19 14:09 Aditya
2020-11-20 19:58 ` [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature Aditya Srivastava
0 siblings, 1 reply; 12+ messages in thread
From: Aditya @ 2020-11-19 14:09 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On 19/11/20 11:23 am, Lukas Bulwahn wrote:
>>>> Goal 1: Try to map all the non-default signatures to their "standard"
>>>> counterpart as much as possible.
>>>>
>>>> Goal 2: Introduce a few very little signatures to handle those cases
>>>> that really cannot be mapped to a non-default signature.
>>>>
>>>> Provide good rationales that you can defend and provide documentation
>>>> for when checkpatch shall explain the fix it proposes.
>>>>
>>>> Here an example for the first ten cases:
>>>>
>>>> 1)Debugged-by: 61 -> Codeveloped-by:
>>>>
>>>> Rationale: Debugging is part of Software Development; so
>>>> Codeveloped-by is perfectly fine, even if the contributor did not
>>>> create code.
>>>>
>>>> (alternatively: maybe a new Assisted-by would do here.)
>>>>
>>>> 2)Requested-by: 48 -> Suggested-by:
>>>>
>>>> Rationale: In an open-source project, there are "no requests", just
>>>> "suggestions" to convince a maintainer to accept your patch.
>>>>
>>>> 3)Co-authored-by: 43 -> Codeveloped-by:
>>>>
>>>> Rationale: clear. Codeveloped-by and Co-authored-by are synonyms.
>>>>
>>>> 4)Originally-by: 39
>>>>
>>>> Maybe something like this deserves to be a new tag. There is a
>>>> significant difference to codeveloped-by. But that needs discussion.
>>>>
>>>> 5)Analyzed-by: 22
>>>>
>>>> Rationale: Analyzing is part of Software Development; so
>>>> Codeveloped-by is perfectly fine, even if the contributor did not
>>>> create code.
>>>> (alternatively: maybe a new Assisted-by would do here.)
>>>>
>>>> 6)Bisected-by: 20
>>>>
>>>> Difficult...
>>>> (maybe a new Assisted-by would do here.)
>>>>
>>>> 7)Improvements-by: 19 -> Codeveloped-by:
>>>>
>>>> 8)Generated-by: 17 -> Reported-by: ?
So, I checked mailing list. Generated-by is used by the user to quote
script(s) and not the person.
E.g., Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
Maybe, it should be suggested to delete this tag.
What do you think?
>>>>
>>>> What does generated-by actually mean?
>>>>
>>>> 9)Noticed-by: 11 -> Reported-by:
>>>>
>>>> 10)Inspired-by: 11 -> Suggested-by:
>>>>
>>>> Maybe you can come up with a list for the next twenty and then we
>>>> discuss them with Joe Perches and then a larger group?
>>>>
>>
>> This is the list for next 20:
>>
>> 11)Original-patch-by: 11 -> co-developed-by / Originally-by (a new
>> signoff)
>> Rationale: I checked mailing list for one of these signoffs.
>> Link1:
>> https://lore.kernel.org/linux-perf-users/20190221122306.1511-1-jonas.rabenstein@studium.uni-erlangen.de/
>> Link2:
>> https://lore.kernel.org/linux-perf-users/20190307174433.28819-32-acme@kernel.org/
>>
>> Here it seems like someone who started working on the patch but
>> couldn't complete it, but still has
>> significant contribution in the patch.
>> Maybe signing off as codeveloper suffices the purpose. I'm not sure though
>>
>
> Agree, that is up for discussion. Either co-developed-by or one new tag.
>
>> 12)Diagnosed-by: 11 -> Maybe 'Reviewed-by' or 'Acked-by'
>> Rationale: Observed a few mailing lists, eg here:
>> https://lore.kernel.org/lkml/20190609164128.000227333@linuxfoundation.org/
>> But could not decide as the user is not adding it along the mails, but
>> seems like a maintainer.
>>
>
> I do not think Acked-by, maybe co-developed-by or reviewed-by.
>
>> 13)Based-on-a-patch-by: 8 -> Similar to 'Originally-by'
>>
>> 14)Verified-by: 8 -> Tested-by
>> Rationale: Used by a single user. On reading, mailing list, it seems
>> that 'Tested-by' tag might be a suitable alternative.
>> Link:
>> https://lore.kernel.org/lkml/CA+jURcugFhSt9GGRZELQUCnupOf2Ns96Ao5ZruWfVtq=z_7ytw@mail.gmail.com/
>>
>
> Agree.
>
>> 15)Okay-ished-by: 8 -> Acked-by
>> Rationale: Used by a single user. On reading, mailing list, it seems
>> that 'Acked-by' tag might be a suitable alternative.
>> Link:
>> https://lore.kernel.org/lkml/f06e74e9a38b83ec273196bce727295b828c5870.1507769413.git.rgb@redhat.com/
>>
>
> Agree.
>
>> 16)Based-on-patch-by: 7 -> Similar to (13) Based-on-a-patch-by
>>
>
> Agree.
>
>> 17)Root-caused-by: 6 -> Maybe 'Fixes:' followed by the commit it is
>> fixing.
>> Rationale: Going through mailing list, it comes up added with the
>> patch. So I couldn't be sure
>>
>
> Hmm... you need to show me the cases where this tag is used.
>
These are some of the examples:
https://lore.kernel.org/lkml/20200904120257.464056467@linuxfoundation.org/
https://lore.kernel.org/lkml/20200904120257.464056467@linuxfoundation.org/
https://lore.kernel.org/lkml/20190507053235.29900-78-sashal@kernel.org/
Not sure if it is used as git blame or maybe to define the 'root' of
the tree at the time, etc
> If the tag is not followed by an identity (name + email), it should
> not be a signature tag anyway.
>
>> 18)Original-by: 6 -> Similar to '(4)Originally-by'
>>
>
> Agree.
>
>> 19)Acked-for-MFD-by: 6 -> Acked-by:
>>
>
> Agree.
>
>> 20)Reviewed-off-by: 5 -> Reviewed-by:
>>
>
> Agree.
>
>> 21)Based-on-patches-by: 5 -> Similar to (13)
>>
>
> Agree.
>
>> 22)Analysed-by: 5 -> Co-developed-by/Reviewed-by
>> Rationale: Similar to '(5)Analyzed-by'
>>
>
> Agree.
>
>> 23)Based-on-work-by: 5 -> Not sure. Maybe 'Suggested-by'
>>
>
> Or similar to 13?
>
Yes, agree.
>> 24)Proposed-by: 5 -> Maybe 'Suggested-by'
>> Rationale: The tag comes up added with the patch,and the user is also
>> given the tag 'Signed-off-by', but does not seem to participate in the
>> conversation.
>> Maybe he is a maintainer, who suggested the patch.
>> mailing list:
>> https://lore.kernel.org/linux-nvme/20200501212545.21856-3-sagi@grimberg.me/
>>
>
> Agree.
>
>> 25)Reported-and-bisected-by: 4 -> Two different tags: 'Reported-by:'
>> and 'Bisected-by'
>>
>
> Agree.
>
>> 26)Fixed-by: 3 -> Co-developed-by
>> Rationale: I observed one of these commit conservations here:
>> https://lore.kernel.org/lkml/1b45ffd1-99bb-4ac1-fb65-0de3e42c1c0a@amd.com/
>> It seems like there was some bug with this patch, which was fixed by
>> the user. I guess Co-developed-by should go well as alternative.
>>
>
> Agree.
>
>> 27)Pointed-out-by: 3 -> Suggested-by
>> Rationale: For commit 87bd4c26a6c8 ("clocksource/drivers/tegra: Lower
>> clocksource rating for some Tegra's"), this warning occurs, where
>> the patch is also 'Acked-by' Peter De Schrijver. So, it seems like he
>> is a maintainer who must have suggested these changes
>>
>
> Agree.
>
>> 28)Suggestions-by: 3 -> Suggested-by
>>
>
> Agree.
>
>> 29)Celebrated-by: 3 -> Might be suggested to remove
>> Rationale: This tag is used for a single commit 3 times, seems like a
>> tag used for celebration of a particular patch
>> Link:
>> https://lore.kernel.org/lkml/CANRm+CyonYOzGdXo+D8gr8n04=f=S92QH-HxETKnoGGxhMFREA@mail.gmail.com/
>>
>
> Agree. Let us suggest deleting such tags.
>
>> 30)Pointed-at-by: 2 -> Suggested-by
>> Rationale: One of these tags is named for Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org>, who is probably a maintainer.
>> Here, the user might just want to acknowledge him for his suggestion,
>> so 'Suggested-by' seems appropriate.
>>
>
> Agree.
>
>> What do you think?
>>
>
> Can you start to implement a patch that creates the basic logic to let
> checkpatch.pl suggest the alternatives for those 30 cases above.
>
Okay. One doubt though, do I need to create a separate patch for edit
distance fix? I am planning to create a hash for these typos(which do
not fulfill edit distance criteria) in checkpatch.pl itself.
What do you think?
> Also, it might be good if checkpatch.pl also provides the explanation
> why that alternative is proposed (when it is not totally obvious).
>
> Can you also summarize which of the 30 cases need a further discussion with Joe?
>
Debugged-by: co-developed-by/reviewed-by/new tag
Originally-by: Co-developed-by / maybe a new tag
Bisected-by
Diagnosed-by
Root-caused-by: not sure if it is used as git blame or maybe to define
the 'root' of the tree at the time, etc
> Lukas
>
_______________________________________________
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] 12+ messages in thread* [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-19 14:09 [Linux-kernel-mentees] Fix for BAD_SIGN_OFF: " Aditya
@ 2020-11-20 19:58 ` Aditya Srivastava
2020-11-20 20:03 ` Aditya
0 siblings, 1 reply; 12+ messages in thread
From: Aditya Srivastava @ 2020-11-20 19:58 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees, yashsri421
Checkpatch.pl warns on non-standard signature styles.
E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
memory leak in ioremap()/iounmap()") reports this warning:
WARNING: Non-standard signature: Noticed-by:
Noticed-by: Sven Schnelle <svens@stackframe.org>
Provide a fix by:
1) replacing the non-standard signature with its standard equivalent
2) removing the signature if it is not required
Also, improve warning messages correspondingly, providing users
suggestions to either replace or remove the signature
Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..23a21dc2c29a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
Cc:
)};
+our %standard_signature_fix = (
+ "Requested-by:" => "Suggested-by:",
+ "Co-authored-by:" => "Co-developed-by:",
+ "Analyzed-by:" => "Co-developed-by:",
+ "Analysed-by:" => "Co-developed-by:",
+ "Improvements-by:" => "Co-developed-by:",
+ "Noticed-by:" => "Reported-by:",
+ "Inspired-by:" => "Suggested-by:",
+ "Verified-by:" => "Tested-by:",
+ "Okay-ished-by:" => "Acked-by:",
+ "Acked-for-MFD-by:" => "Acked-by:",
+ "Reviewed-off-by:" => "Reviewed-by:",
+ "Proposed-by:" => "Suggested-by:",
+ "Fixed-by:" => "Co-developed-by:",
+ "Pointed-out-by:" => "Suggested-by:",
+ "Pointed-at-by:" => "Suggested-by:",
+ "Suggestions-by:" => "Suggested-by:",
+ "Generated-by:" => "remove",
+ "Celebrated-by:" => "remove",
+);
+
our @typeListMisordered = (
qr{char\s+(?:un)?signed},
qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2794,28 @@ sub process {
my $ucfirst_sign_off = ucfirst(lc($sign_off));
if ($sign_off !~ /$signature_tags/) {
- WARN("BAD_SIGN_OFF",
- "Non-standard signature: $sign_off\n" . $herecurr);
+ my $suggested_signature = "";
+ if (exists($standard_signature_fix{$sign_off})) {
+ $suggested_signature = $standard_signature_fix{$sign_off};
+ }
+ if ($suggested_signature eq "") {
+ WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off\n" . $herecurr);
+ }
+ elsif ($suggested_signature eq "remove") {
+ if (WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
+ $fix) {
+ fix_delete_line($fixlinenr, $rawline);
+ }
+ }
+ else {
+ if (WARN("BAD_SIGN_OFF",
+ "Non-standard signature: $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+ }
+ }
}
if (defined $space_before && $space_before ne "") {
if (WARN("BAD_SIGN_OFF",
--
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] 12+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-20 19:58 ` [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature Aditya Srivastava
@ 2020-11-20 20:03 ` Aditya
2020-11-20 20:23 ` Lukas Bulwahn
0 siblings, 1 reply; 12+ messages in thread
From: Aditya @ 2020-11-20 20:03 UTC (permalink / raw)
To: lukas.bulwahn; +Cc: linux-kernel-mentees
On 21/11/20 1:28 am, Aditya Srivastava wrote:
> Checkpatch.pl warns on non-standard signature styles.
>
> E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
> memory leak in ioremap()/iounmap()") reports this warning:
>
> WARNING: Non-standard signature: Noticed-by:
> Noticed-by: Sven Schnelle <svens@stackframe.org>
>
> Provide a fix by:
> 1) replacing the non-standard signature with its standard equivalent
> 2) removing the signature if it is not required
>
> Also, improve warning messages correspondingly, providing users
> suggestions to either replace or remove the signature
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..23a21dc2c29a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
> Cc:
> )};
>
> +our %standard_signature_fix = (
> + "Requested-by:" => "Suggested-by:",
> + "Co-authored-by:" => "Co-developed-by:",
> + "Analyzed-by:" => "Co-developed-by:",
> + "Analysed-by:" => "Co-developed-by:",
> + "Improvements-by:" => "Co-developed-by:",
> + "Noticed-by:" => "Reported-by:",
> + "Inspired-by:" => "Suggested-by:",
> + "Verified-by:" => "Tested-by:",
> + "Okay-ished-by:" => "Acked-by:",
> + "Acked-for-MFD-by:" => "Acked-by:",
> + "Reviewed-off-by:" => "Reviewed-by:",
> + "Proposed-by:" => "Suggested-by:",
> + "Fixed-by:" => "Co-developed-by:",
> + "Pointed-out-by:" => "Suggested-by:",
> + "Pointed-at-by:" => "Suggested-by:",
> + "Suggestions-by:" => "Suggested-by:",
> + "Generated-by:" => "remove",
> + "Celebrated-by:" => "remove",
> +);
> +
> our @typeListMisordered = (
> qr{char\s+(?:un)?signed},
> qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2794,28 @@ sub process {
> my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
> if ($sign_off !~ /$signature_tags/) {
> - WARN("BAD_SIGN_OFF",
> - "Non-standard signature: $sign_off\n" . $herecurr);
> + my $suggested_signature = "";
> + if (exists($standard_signature_fix{$sign_off})) {
> + $suggested_signature = $standard_signature_fix{$sign_off};
> + }
> + if ($suggested_signature eq "") {
> + WARN("BAD_SIGN_OFF",
> + "Non-standard signature: $sign_off\n" . $herecurr);
> + }
> + elsif ($suggested_signature eq "remove") {
> + if (WARN("BAD_SIGN_OFF",
> + "Non-standard signature: $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
> + $fix) {
> + fix_delete_line($fixlinenr, $rawline);
> + }
> + }
> + else {
> + if (WARN("BAD_SIGN_OFF",
> + "Non-standard signature: $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
> + $fix) {
> + $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> + }
> + }
> }
> if (defined $space_before && $space_before ne "") {
> if (WARN("BAD_SIGN_OFF",
>
Initial tests performed on patches found this fix to be working as
expected.
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] 12+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-20 20:03 ` Aditya
@ 2020-11-20 20:23 ` Lukas Bulwahn
2020-11-20 21:30 ` Aditya
0 siblings, 1 reply; 12+ messages in thread
From: Lukas Bulwahn @ 2020-11-20 20:23 UTC (permalink / raw)
To: Aditya; +Cc: linux-kernel-mentees
[-- Attachment #1.1: Type: text/plain, Size: 4469 bytes --]
On Fr., 20. Nov. 2020 at 21:03, Aditya <yashsri421@gmail.com> wrote:
> On 21/11/20 1:28 am, Aditya Srivastava wrote:
> > Checkpatch.pl warns on non-standard signature styles.
> >
> > E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
> > memory leak in ioremap()/iounmap()") reports this warning:
> >
> > WARNING: Non-standard signature: Noticed-by:
> > Noticed-by: Sven Schnelle <svens@stackframe.org>
> >
This example really does not tell anyone much.
Replace it with a summary from your evaluation.
> > Provide a fix by:
> > 1) replacing the non-standard signature with its standard equivalent
> > 2) removing the signature if it is not required
> >
> > Also, improve warning messages correspondingly, providing users
> > suggestions to either replace or remove the signature
> >
Looks good.
> > Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> > ---
> > scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index fdfd5ec09be6..23a21dc2c29a 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
> > Cc:
> > )};
> >
> > +our %standard_signature_fix = (
> > + "Requested-by:" => "Suggested-by:",
> > + "Co-authored-by:" => "Co-developed-by:",
> > + "Analyzed-by:" => "Co-developed-by:",
> > + "Analysed-by:" => "Co-developed-by:",
> > + "Improvements-by:" => "Co-developed-by:",
> > + "Noticed-by:" => "Reported-by:",
> > + "Inspired-by:" => "Suggested-by:",
> > + "Verified-by:" => "Tested-by:",
> > + "Okay-ished-by:" => "Acked-by:",
> > + "Acked-for-MFD-by:" => "Acked-by:",
> > + "Reviewed-off-by:" => "Reviewed-by:",
> > + "Proposed-by:" => "Suggested-by:",
> > + "Fixed-by:" => "Co-developed-by:",
> > + "Pointed-out-by:" => "Suggested-by:",
> > + "Pointed-at-by:" => "Suggested-by:",
> > + "Suggestions-by:" => "Suggested-by:",
> > + "Generated-by:" => "remove",
> > + "Celebrated-by:" => "remove",
> > +);
> > +
How did create this list? I thought we looked at 30 cases...
> > our @typeListMisordered = (
> > qr{char\s+(?:un)?signed},
> > qr{int\s+(?:(?:un)?signed\s+)?short\s},
> > @@ -2773,8 +2794,28 @@ sub process {
> > my $ucfirst_sign_off = ucfirst(lc($sign_off));
> >
> > if ($sign_off !~ /$signature_tags/) {
> > - WARN("BAD_SIGN_OFF",
> > - "Non-standard signature: $sign_off\n"
> . $herecurr);
> > + my $suggested_signature = "";
> > + if
> (exists($standard_signature_fix{$sign_off})) {
> > + $suggested_signature =
> $standard_signature_fix{$sign_off};
> > + }
> > + if ($suggested_signature eq "") {
> > + WARN("BAD_SIGN_OFF",
> > + "Non-standard signature:
> $sign_off\n" . $herecurr);
> > + }
> > + elsif ($suggested_signature eq "remove") {
> > + if (WARN("BAD_SIGN_OFF",
> > + "Non-standard signature:
> $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
> > + $fix) {
> > +
> fix_delete_line($fixlinenr, $rawline);
> > + }
> > + }
> > + else {
> > + if (WARN("BAD_SIGN_OFF",
> > + "Non-standard signature:
> $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
> > + $fix) {
> > + $fixed[$fixlinenr] =~
> s/$sign_off/$suggested_signature/;
> > + }
> > + }
> > }
> > if (defined $space_before && $space_before ne "") {
> > if (WARN("BAD_SIGN_OFF",
> >
>
> Initial tests performed on patches found this fix to be working as
> expected.
>
> Thanks
> Aditya
>
[-- Attachment #1.2: Type: text/html, Size: 7527 bytes --]
[-- Attachment #2: Type: text/plain, Size: 201 bytes --]
_______________________________________________
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] 12+ messages in thread* Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature
2020-11-20 20:23 ` Lukas Bulwahn
@ 2020-11-20 21:30 ` Aditya
0 siblings, 0 replies; 12+ messages in thread
From: Aditya @ 2020-11-20 21:30 UTC (permalink / raw)
To: Lukas Bulwahn; +Cc: linux-kernel-mentees
On 21/11/20 1:53 am, Lukas Bulwahn wrote:
> On Fr., 20. Nov. 2020 at 21:03, Aditya <yashsri421@gmail.com> wrote:
>
>> On 21/11/20 1:28 am, Aditya Srivastava wrote:
>>> Checkpatch.pl warns on non-standard signature styles.
>>>
>>> E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
>>> memory leak in ioremap()/iounmap()") reports this warning:
>>>
>>> WARNING: Non-standard signature: Noticed-by:
>>> Noticed-by: Sven Schnelle <svens@stackframe.org>
>>>
>
>
> This example really does not tell anyone much.
>
> Replace it with a summary from your evaluation.
>
Okay
>
>>> Provide a fix by:
>>> 1) replacing the non-standard signature with its standard equivalent
>>> 2) removing the signature if it is not required
>>>
>>> Also, improve warning messages correspondingly, providing users
>>> suggestions to either replace or remove the signature
>>>
>
>
> Looks good.
>
>
>
>>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>>> ---
>>> scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
>>> 1 file changed, 43 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index fdfd5ec09be6..23a21dc2c29a 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
>>> Cc:
>>> )};
>>>
>>> +our %standard_signature_fix = (
>>> + "Requested-by:" => "Suggested-by:",
>>> + "Co-authored-by:" => "Co-developed-by:",
>>> + "Analyzed-by:" => "Co-developed-by:",
>>> + "Analysed-by:" => "Co-developed-by:",
>>> + "Improvements-by:" => "Co-developed-by:",
>>> + "Noticed-by:" => "Reported-by:",
>>> + "Inspired-by:" => "Suggested-by:",
>>> + "Verified-by:" => "Tested-by:",
>>> + "Okay-ished-by:" => "Acked-by:",
>>> + "Acked-for-MFD-by:" => "Acked-by:",
>>> + "Reviewed-off-by:" => "Reviewed-by:",
>>> + "Proposed-by:" => "Suggested-by:",
>>> + "Fixed-by:" => "Co-developed-by:",
>>> + "Pointed-out-by:" => "Suggested-by:",
>>> + "Pointed-at-by:" => "Suggested-by:",
>>> + "Suggestions-by:" => "Suggested-by:",
>>> + "Generated-by:" => "remove",
>>> + "Celebrated-by:" => "remove",
>>> +);
>>> +
>
>
> How did create this list? I thought we looked at 30 cases...
>
>
Yes, correct. Others are the cases which required discussion like:
"Originally-by" and its variants (including 'Based-on-patch-by', etc),
"Bisected-by", "Diagnosed-by", "Root-caused-by".
Thanks
Aditya
>>> our @typeListMisordered = (
>>> qr{char\s+(?:un)?signed},
>>> qr{int\s+(?:(?:un)?signed\s+)?short\s},
>>> @@ -2773,8 +2794,28 @@ sub process {
>>> my $ucfirst_sign_off = ucfirst(lc($sign_off));
>>>
>>> if ($sign_off !~ /$signature_tags/) {
>>> - WARN("BAD_SIGN_OFF",
>>> - "Non-standard signature: $sign_off\n"
>> . $herecurr);
>>> + my $suggested_signature = "";
>>> + if
>> (exists($standard_signature_fix{$sign_off})) {
>>> + $suggested_signature =
>> $standard_signature_fix{$sign_off};
>>> + }
>>> + if ($suggested_signature eq "") {
>>> + WARN("BAD_SIGN_OFF",
>>> + "Non-standard signature:
>> $sign_off\n" . $herecurr);
>>> + }
>>> + elsif ($suggested_signature eq "remove") {
>>> + if (WARN("BAD_SIGN_OFF",
>>> + "Non-standard signature:
>> $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
>>> + $fix) {
>>> +
>> fix_delete_line($fixlinenr, $rawline);
>>> + }
>>> + }
>>> + else {
>>> + if (WARN("BAD_SIGN_OFF",
>>> + "Non-standard signature:
>> $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
>>> + $fix) {
>>> + $fixed[$fixlinenr] =~
>> s/$sign_off/$suggested_signature/;
>>> + }
>>> + }
>>> }
>>> if (defined $space_before && $space_before ne "") {
>>> if (WARN("BAD_SIGN_OFF",
>>>
>>
>> Initial tests performed on patches found this fix to be working as
>> expected.
>>
>> 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] 12+ messages in thread
end of thread, other threads:[~2020-11-28 13:00 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-24 11:16 [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature Aditya Srivastava
2020-11-24 11:48 ` Lukas Bulwahn
2020-11-24 15:32 ` [Linux-kernel-mentees] [PATCH v2] " Aditya Srivastava
2020-11-25 6:57 ` Lukas Bulwahn
2020-11-25 11:25 ` [Linux-kernel-mentees] [PATCH v3] " Aditya Srivastava
2020-11-25 12:26 ` Lukas Bulwahn
2020-11-28 9:52 ` [Linux-kernel-mentees] [PATCH v4] checkpatch: add fix and improve warning msg for non-standard signature Aditya Srivastava
2020-11-28 13:00 ` Lukas Bulwahn
-- strict thread matches above, loose matches on Subject: below --
2020-11-19 14:09 [Linux-kernel-mentees] Fix for BAD_SIGN_OFF: " Aditya
2020-11-20 19:58 ` [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature Aditya Srivastava
2020-11-20 20:03 ` Aditya
2020-11-20 20:23 ` Lukas Bulwahn
2020-11-20 21:30 ` 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.