[PATCH v2] checkpatch: fix missing whitespace in formatted email

[PATCH v2] checkpatch: fix missing whitespace in formatted email

[Dwaipayan Ray]

Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
checks.") introduced new checks for author sign off. The format_email
procedure was modified to add comment blocks to the formatted email. But
no space was added between the email address and mail comment.

This causes wrong email formatting in cases where the email is in the form
"author@example.com (Comment block)". The space between the address and
comment block is removed, which causes incorrect parsing of the
formatted email.

An evaluation on checkpatch brought up this case. For example,
on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
the following warning was reported:

WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
system concerns)' might be better as 'David.Laight@aculab.com(big endian
system concerns)'

Add a single space in between the address and comment when the
extracted comment is not empty.

Fixes: 0c01921e56f9 ("checkpatch: add new warnings to author signoff checks.")
Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..f1a4e61917eb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1221,7 +1221,9 @@ sub format_email {
 	} else {
 		$formatted_email = "$name$name_comment <$address>";
 	}
-	$formatted_email .= "$comment";
+	if ("$comment" ne "") {
+		$formatted_email .= " $comment";
+	}
 	return $formatted_email;
 }
 
-- 
2.27.0

Re: [PATCH v2] checkpatch: fix missing whitespace in formatted email

[Joe Perches]

On Tue, 2020-10-13 at 12:06 +0530, Dwaipayan Ray wrote:
> Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> checks.") introduced new checks for author sign off. The format_email
> procedure was modified to add comment blocks to the formatted email. But
> no space was added between the email address and mail comment.
> 
> This causes wrong email formatting in cases where the email is in the form
> "author@example.com (Comment block)". The space between the address and
> comment block is removed, which causes incorrect parsing of the
> formatted email.
> 
> An evaluation on checkpatch brought up this case. For example,
> on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
> the following warning was reported:
> 
> WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
> system concerns)' might be better as 'David.Laight@aculab.com(big endian
> system concerns)'

Strictly, a comment or multiple comments can exist
in any part of an email

"John (Jon) Smith (Smitty) <jsmith@domain.tld> (tld is the best tld)

> Add a single space in between the address and comment when the
> extracted comment is not empty.

So after the address is not necessarily the best
location for a comment.

Re: [PATCH v2] checkpatch: fix missing whitespace in formatted email

[Dwaipayan Ray]

On Tue, Oct 13, 2020 at 12:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2020-10-13 at 12:06 +0530, Dwaipayan Ray wrote:
> > Commit 0c01921e56f9 ("checkpatch: add new warnings to author signoff
> > checks.") introduced new checks for author sign off. The format_email
> > procedure was modified to add comment blocks to the formatted email. But
> > no space was added between the email address and mail comment.
> >
> > This causes wrong email formatting in cases where the email is in the form
> > "author@example.com (Comment block)". The space between the address and
> > comment block is removed, which causes incorrect parsing of the
> > formatted email.
> >
> > An evaluation on checkpatch brought up this case. For example,
> > on commit 1129d31b55d5 ("ima: Fix ima digest hash table key calculation"),
> > the following warning was reported:
> >
> > WARNING:BAD_SIGN_OFF: email address 'David.Laight@aculab.com (big endian
> > system concerns)' might be better as 'David.Laight@aculab.com(big endian
> > system concerns)'
>
> Strictly, a comment or multiple comments can exist
> in any part of an email
>
> "John (Jon) Smith (Smitty) <jsmith@domain.tld> (tld is the best tld)
>
> > Add a single space in between the address and comment when the
> > extracted comment is not empty.
>
> So after the address is not necessarily the best
> location for a comment.
>
>

Yes I agree and in cases like that the check works perfectly.
But now consider the case when the name is empty:

Like:
jsmith@domain.tld (tld is the best tld).

In this case when the formatted email is generated, we have
the following: "jsmith@domain.tld(tld is the best tld)".
If this email is parsed again, the entire part is parsed as the email address,
which causes a mismatch later on.

It is for only this case that an extra white space is needed for the
parsing, probably because of how the comment is captured, for emails
with empty names:
  $formatted_email =~ /(\S+\@\S+)(.*)$/
So it depends on the whitespace after address to demarcate the comment.

Some more examples of what I intend to solve:
'<stable@vger.kernel.org> [5.3+]'
'<stable@vger.kernel.org> # v5.7+'
'David.Laight@aculab.com (big endian system concerns)'


Thanks,
Dwaipayan.