Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: extend author Signed-off-by check for split From: header

[Lukas Bulwahn <lukas.bulwahn@gmail.com>]

On Sun, 20 Sep 2020, Dwaipayan Ray wrote:

> On Sun, Sep 20, 2020 at 2:18 AM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
> >
> > Checkpatch did not handle cases where the author From: header
> > was split into multiple lines. The author identity could not
> > be resolved and checkpatch generated a false NO_AUTHOR_SIGN_OFF
> > warning.
> >
> > Support split From: headers in AUTHOR_SIGN_OFF check by correctly
> > parsing the entire From: header in such cases.
> >
> > Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> > ---
> >  scripts/checkpatch.pl | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 504d2e431c60..8ee61ec346b3 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -2661,6 +2661,10 @@ sub process {
> >  # Check the patch for a From:
> >                 if (decode("MIME-Header", $line) =~ /^From:\s*(.*)/) {
> >                         $author = $1;
> > +                       my $curline = $linenr-1;
> > +                       while(defined($rawlines[++$curline]) && ($rawlines[$curline] =~ /^\s+(.*)/)) {
> > +                               $author.= $1;
> > +                       }
> >                         $author = encode("utf8", $author) if ($line =~ /=\?utf-8\?/i);
> >                         $author =~ s/"//g;
> >                         $author = reformat_email($author);
> > --
> > 2.27.0
> >
> 
> Hi,
> Based on Joe's suggestions I updated the patch. It's totally similar
> except for the fact that Joe missed a closing parenthesis and
> he removed the decode("MIME-Header") part which caused parsing
> of utf-8 to fail.
> 
> example: (commit e33bcbab16d1)
> WARNING: Missing Signed-off-by: line by nominal patch author
> '"=?UTF-8?q?Vesa=20J=C3=A4=C3=A4skel=C3=A4inen?="
> <vesa.jaaskelainen@vaisala.com>
> 
> So I fixed these.
> 
> Also, Joe stated that all split lines must start with a space. But I
> looked it up and it states that it must be a WSP, that is either a
> single space or a horizontal tab.
> References:
> WSP: (https://www.rfc-editor.org/std/std68.txt)
> Long headers: https://tools.ietf.org/html/rfc2822#section-2.2.3
> 
> So, i went with the ^\s+ expression instead of ^ s*. Is it alright
> or should be changed?
> 
> I do have a final question though, since he had already solved it,
> is it okay to send this patch in again?
>

Yes, please provide your design decisions in the commit message.

You can add a 'Suggested-by:' tag to name that Joe pointed out the idea.

Also, you can add a 'Link:' to the previous email from Joe with a proper 
lore.kernel.org hyperlink.

Just send out your PATCH v2, then we can all review and test it.

When we have this patch accepted, we can discuss with Joe some way to 
handle the further cases of false warnings in AUTHOR_SIGN_OFF.


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