(adding stable and Greg KH for additional review) On Thu, 2020-11-05 at 17:29 +0530, Dwaipayan Ray wrote:

checkpatch doesn't report warnings for many common mistakes in emails. Some of which are trailing commas and incorrect use of email comments.

I presume you've tested this against the git tree.

Can you send me a file with the BAD_SIGN_OFF messages generated and if possible the git SHA-1s of the commits?

At the same time several false positives are reported due to incorrect handling of mail comments. The most common of which is due to the pattern:

stable@vger.kernel.org # X.X

Improve email parsing in checkpatch.

Some general comment rules are defined:

• Multiple name comments should not be allowed.
• Comments inside address should not be allowed.
• In general comments should be enclosed within parentheses.

Exception for stable@vger.kernel.org # X.X

not just vger.kernel.org, but this should also allow stable@kernel.org and only allow cc: and not any other -by: type for that email address.

A process preference question for Greg and the stable team:

The most common stable forms are

stable@vger.kernel.org # version info then stable@vger.kernel.org [ version info ]

with some other relatively infrequently used outlier styles, some that use parentheses, but this is not frequent.

It might be sensible to standardize on the "# version info" trailer comment version info style and warn on any other form.

A somewhat common style for the stable address is to use a name before the stable address which describes the version info:

Perhaps any name before stable should be warned and the version should be a comment.

Here's a list of the stable addresses with "version name" then stable address in the git tree and other outlier styles.

24 linux-stable stable@vger.kernel.org 21 5.4+ stable@vger.kernel.org 14 All applicable stable@vger.kernel.org 6 3.10+ stable@vger.kernel.org 5 5.9+ stable@vger.kernel.org 5 5.3+ stable@vger.kernel.org 5 5.1+ stable@vger.kernel.org 4 5.6+ stable@vger.kernel.org 4 4.20+ stable@vger.kernel.org 3 Stable Team stable@vger.kernel.org 3 4.19+ stable@vger.kernel.org 3 4.15+ stable@vger.kernel.org 3 4.10+ stable@vger.kernel.org 2 stable@vger.kernel.org (v2.6.12+) 2 5.2+ stable@vger.kernel.org 2 4.16+ stable@vger.kernel.org 1 v5.8+ stable@vger.kernel.org 1 v5.7+ stable@vger.kernel.org 1 v5.6+ stable@vger.kernel.org 1 v5.3+ stable@vger.kernel.org 1 v5.0+ stable@vger.kernel.org 1 v4.9+ stable@vger.kernel.org 1 stable@vger.kernel.org v5.0+ 1 stable@vger.kernel.org +v4.18 1 stable@vger.kernel.org (3.14+) 1 5.8+ stable@vger.kernel.org 1 5.5+ stable@vger.kernel.org 1 5.0+ stable@vger.kernel.org 1 4.18+ stable@vger.kernel.org 1 4.14+ stable@vger.kernel.org 1 4.13+ stable@vger.kernel.org 1 4.0+ stable@vger.kernel.org 1 3.15+ stable@vger.kernel.org 1 3.11+ stable@vger.kernel.org

Improvements to parsing:

• Detect and report unexpected content after email.
• Quoted names are excluded from comment parsing.
• Trailing dots or commas in email are removed during

formatting. Correspondingly a BAD_SIGN_OFF warning   is emitted.

• Improperly quoted email like '"name <address>"' are now

warned about.

All of the above seems right but perhaps the comment style for any <foo>-by: lines should also allow # comments.

The below is just comments on the patch itself.

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl

[]

@@ -2800,9 +2806,57 @@ sub process {   $dequoted =~ s/" </ </;   # Don't force email to have quotes   # Allow just an angle bracketed address

• 		if (!same_email_addresses($email, $suggested_email, 0)) {
  

• 		if (!same_email_addresses($email, $suggested_email)) {
  

• 			if (WARN("BAD_SIGN_OFF",
  

• 			    "email address '$email' might be better as '$suggested_email'\n" . $herecurr) &&
  

• 				$fix) {
  

trivia:

Please always align $fix with tabs to the if and then 4 spaces to the open parenthesis.

• 		# Comments must begin only with (
  

• 		# or # in case of stable@vger.kernel.org
  

• 		if ($email =~ /^.*stable\@vger/) {
  

I believe this should be

if ($email =~ /^stable@(?:vger.)?kernel.org$/) {

• 			if ($comment ne "" && $comment !~ /^#.+/) {
  

• 				if (WARN("BAD_SIGN_OFF",
  

• 				    "Invalid comment format for stable: '$email', prefer parentheses\n" . $herecurr) &&
  

Prefer #

• 					$fix) {
  

• 					my $new_comment = $comment;
  

• 					$new_comment =~ s/^[ \(\[]+|[ \)\]]+$//g;
  

Does the comment include any leading whitespace here? I presumed not given the $comment !~ /^#/ test above.