Re: [PATCH v3] checkpatch: add fix option for MISSING_SIGN_OFF

[Aditya <yashsri421@gmail.com>]

On 30/11/20 2:45 pm, Aditya Srivastava wrote:
> Currently checkpatch warns us if there is no 'Signed-off-by' line for
> the patch.
> 
> E.g., running checkpatch on commit 9ac060a708e0 ("leaking_addresses:
> Completely remove --version flag") reports this error:
> 
> ERROR: Missing Signed-off-by: line(s)
> 
> Provide a fix by adding a Signed-off-by line corresponding to the author
> of the patch before the patch separator line.
> 
> Also, avoid this fix with the Non-standard signature warning, as the
> missing sign off is most likely because of typo.
> 
> E.g. for commit 8cde5d5f7361 ("bus: ti-sysc: Detect omap4 type timers
> for quirk") we get missing sign off as well as bad sign off warnings for:
> 
> Siganed-off-by: Tony Lindgren <tony@atomide.com>
> 
> Here it is probably best to fix the typo with BAD_SIGN_OFF warning and
> avoid adding an additional signoff.
> 
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> applies over next-20201120 and my last changes at:https://lore.kernel.org/linux-kernel-mentees/db1195235752685fc85fb52ecb1b1af3f35b5394.camel@perches.com/T/#u
> 
> Changes in v2:
>   Add space after 'if'
>   Add check for $patch_separator_linenr to be greater than 0
> 
> Changes in v3:
>   Give MISSING_SIGN_OFF warning irrespective of the value of $non_standard_signature, add check with fix option instead
>   Modify commit message accordingly
> 
>  scripts/checkpatch.pl | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 4a026926139f..3abf34bb9b00 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2462,6 +2462,8 @@ sub process {
>  
>  	my $last_blank_line = 0;
>  	my $last_coalesced_string_linenr = -1;
> +	my $patch_separator_linenr = 0;
> +	my $non_standard_signature = 0;
>  
>  	our @report = ();
>  	our $cnt_lines = 0;
> @@ -2813,6 +2815,10 @@ sub process {
>  		if ($line =~ /^---$/) {
>  			$has_patch_separator = 1;
>  			$in_commit_log = 0;
> +			# to add missing signed-off-by line before diff(s)
> +			if ($patch_separator_linenr == 0) {
> +				$patch_separator_linenr = $linenr;
> +			}
>  		}
>  
>  # Check if MAINTAINERS is being updated.  If so, there's probably no need to
> @@ -2842,6 +2848,7 @@ sub process {
>  						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
>  					}
>  				}
> +				$non_standard_signature = 1;
>  			}
>  			if (defined $space_before && $space_before ne "") {
>  				if (WARN("BAD_SIGN_OFF",
> @@ -7188,8 +7195,11 @@ sub process {
>  	}
>  	if ($is_patch && $has_commit_log && $chk_signoff) {
>  		if ($signoff == 0) {
> -			ERROR("MISSING_SIGN_OFF",
> -			      "Missing Signed-off-by: line(s)\n");
> +			if (ERROR("MISSING_SIGN_OFF",
> +				  "Missing Signed-off-by: line(s)\n") &&
> +			    $fix && !$non_standard_signature && $patch_separator_linenr > 0) {
> +				fix_insert_line($patch_separator_linenr - 1, "Signed-off-by: $author");
> +			}
>  		} elsif ($authorsignoff != 1) {
>  			# authorsignoff values:
>  			# 0 -> missing sign off
> 

In reply to:
https://lore.kernel.org/linux-kernel-mentees/f21acc492ab37acc42390abffb61aed370a22118.camel@perches.com/

Thanks
Aditya