lkml.org 
[lkml]   [2020]   [Sep]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH v2] checkpatch: extend author Signed-off-by check for split From: header
From
Date
On Sun, 2020-09-20 at 14:47 +0530, Dwaipayan Ray 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.

Hi Dwaipayan.

> A typical example is Commit e33bcbab16d1 ("tee: add support for
> session's client UUID generation"). When checkpatch was run on
> this commit, it displayed:
>
> "WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal
> patch author ''"
>
> This was due to split header lines not being handled properly and
> the author himself wrote in Commit cd2614967d8b ("checkpatch: warn
> if missing author Signed-off-by"):
>
> "Split From: headers are not fully handled: only the first part
> is compared."
>
> Support split From: headers by correctly parsing the header
> extension lines. RFC 2822, Section-2.2.3 stated that each extended
> line must start with a WSP character (a space or htab). The solution
> was therefore to concatenate the lines which start with a WSP to
> get the correct long header.

This is a good commit message, though I believe the
latest rfc is 5322. I'm not sure there is any real
difference in the referenced section though.

While your patch seems to work for git format-email,
other emailers seem to set headers that have multiple
whitespace chars that should be collapsed into a
single space.

I think you'll find that the eliding all whitespace
after header folding causes mismatches for emails.

For instance:

From: "=?UTF-8?q?Christian=20K=C3=B6nig?="
<ckoenig.leichtzumerken@gmail.com>

Always inserting a single space if there is any
whitespace after the folding WSP might be better
otherwise this is decoded as

From: "Christian König"<ckoenig.leichtzumerken@gmail.com>

What I have does a bit more by saving any post-folding

"From: <name and email address>"

and comparing that to any "name and perhaps different
email address" in a Signed-off-by: line.

A new message is emitted if the name matches but the
email address is different.

Perhaps it's reasonable to apply your patch and then
update it with something like the below:
---
scripts/checkpatch.pl | 32 ++++++++++++++++++++++++++++----
1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3e474072aa90..1ecc179e938d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1240,6 +1240,15 @@ sub same_email_addresses {
$email1_address eq $email2_address;
}

+sub same_email_names {
+ my ($email1, $email2) = @_;
+
+ my ($email1_name, $name1_comment, $email1_address, $comment1) = parse_email($email1);
+ my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2);
+
+ return $email1_name eq $email2_name;
+}
+
sub which {
my ($bin) = @_;

@@ -2679,20 +2688,32 @@ sub process {
}

# Check the patch for a From:
- if (decode("MIME-Header", $line) =~ /^From:\s*(.*)/) {
+ if ($line =~ /^From:\s*(.*)/i) {
$author = $1;
- $author = encode("utf8", $author) if ($line =~ /=\?utf-8\?/i);
+ my $curline = $linenr;
+ while (defined($rawlines[$curline]) && $rawlines[$curline++] =~ /^\s(\s+)?(.*)/) {
+ $author .= ' ' if (defined($1));
+ $author .= "$2";
+ }
+ if ($author =~ /=\?utf-8\?/i) {
+ $author = decode("MIME-Header", $author);
+ $author = encode("utf8", $author);
+ }
+
$author =~ s/"//g;
$author = reformat_email($author);
}

# Check the patch for a signoff:
if ($line =~ /^\s*signed-off-by:\s*(.*)/i) {
+ my $sig = $1;
$signoff++;
$in_commit_log = 0;
if ($author ne '') {
- if (same_email_addresses($1, $author)) {
- $authorsignoff = 1;
+ if (same_email_addresses($sig, $author)) {
+ $authorsignoff = "1";
+ } elsif (same_email_names($sig, $author)) {
+ $authorsignoff = $sig;
}
}
}
@@ -6937,6 +6958,9 @@ sub process {
} elsif (!$authorsignoff) {
WARN("NO_AUTHOR_SIGN_OFF",
"Missing Signed-off-by: line by nominal patch author '$author'\n");
+ } elsif ($authorsignoff ne "1") {
+ WARN("NO_AUTHOR_SIGN_OFF",
+ "From:/SoB: email address mismatch: 'From: $author' != 'Signed-off-by: $authorsignoff'\n");
}
}

\
 
 \ /
  Last update: 2020-09-20 17:11    [W:0.126 / U:1.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site