[Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature

Aditya yashsri421 at gmail.com
Fri Nov 20 21:30:09 UTC 2020


On 21/11/20 1:53 am, Lukas Bulwahn wrote:
> On Fr., 20. Nov. 2020 at 21:03, Aditya <yashsri421 at 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 at 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 at 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
>>
> 



More information about the Linux-kernel-mentees mailing list