[PATCH v4] checkpatch: improve email parsing

[PATCH v4] checkpatch: improve email parsing

[Dwaipayan Ray]

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

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 email 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.
  Relaxation is given to comments beginning with #.
- Stable addresses should not begin with a name.
- Comments in stable addresses should begin only
  with a #.

Improvements to parsing:

- Detect and report unexpected content after email.
- Quoted names are excluded from comment parsing.
- Trailing dots, commas or quotes in email are removed during
  formatting. Correspondingly a BAD_SIGN_OFF warning
  is emitted.
- Improperly quoted email like '"name <address>"' are now
  warned about.

In addition, added fixes for all the possible rules.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 80 insertions(+), 17 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..d866c4321182 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1152,6 +1152,7 @@ sub parse_email {
 	my ($formatted_email) = @_;
 
 	my $name = "";
+	my $quoted = "";
 	my $name_comment = "";
 	my $address = "";
 	my $comment = "";
@@ -1183,14 +1184,20 @@ sub parse_email {
 		}
 	}
 
-	$comment = trim($comment);
-	$name = trim($name);
-	$name =~ s/^\"|\"$//g;
-	if ($name =~ s/(\s*\([^\)]+\))\s*//) {
-		$name_comment = trim($1);
+	# Extract comments from names excluding quoted parts
+	# "John A. (Kennedy)" - Do not extract
+	if ($name =~ s/\"(.+)\"//) {
+		$quoted = $1;
+	}
+	while ($name =~ s/\s*($balanced_parens)\s*/ /) {
+		$name_comment .= trim($1);
 	}
+	$name =~ s/^[ \"]+|[ \"]+$//g;
+	$name = trim("$quoted $name");
+
 	$address = trim($address);
 	$address =~ s/^\<|\>$//g;
+	$comment = trim($comment);
 
 	if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
 		$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
@@ -1205,17 +1212,20 @@ sub format_email {
 
 	my $formatted_email;
 
-	$name_comment = trim($name_comment);
-	$comment = trim($comment);
-	$name = trim($name);
-	$name =~ s/^\"|\"$//g;
+	$name =~ s/^[ \"]+|[ \"]+$//g;
 	$address = trim($address);
+	$address =~ s/(?:\.|\,|\")+$//; ##trailing commas, dots or quotes
 
 	if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
 		$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
 		$name = "\"$name\"";
 	}
 
+	$name_comment = trim($name_comment);
+	$name_comment = " $name_comment" if length($name_comment) > 0;
+	$comment = trim($comment);
+	$comment = " $comment" if length($comment) > 0;
+
 	if ("$name" eq "") {
 		$formatted_email = "$address";
 	} else {
@@ -1233,15 +1243,11 @@ sub reformat_email {
 }
 
 sub same_email_addresses {
-	my ($email1, $email2, $match_comment) = @_;
+	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);
 
-	if ($match_comment != 1) {
-		return $email1_name eq $email2_name &&
-		       $email1_address eq $email2_address;
-	}
 	return $email1_name eq $email2_name &&
 	       $email1_address eq $email2_address &&
 	       $name1_comment eq $name2_comment &&
@@ -2704,7 +2710,7 @@ sub process {
 			$signoff++;
 			$in_commit_log = 0;
 			if ($author ne ''  && $authorsignoff != 1) {
-				if (same_email_addresses($1, $author, 1)) {
+				if (same_email_addresses($1, $author)) {
 					$authorsignoff = 1;
 				} else {
 					my $ctx = $1;
@@ -2800,9 +2806,66 @@ 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) {
+						$fixed[$fixlinenr] =~ s/\Q$email\E/$suggested_email/;
+					}
+				}
+
+				# Address part shouldn't have comments
+				my $stripped_address = $email_address;
+				$stripped_address =~ s/\([^\(\)]*\)//g;
+				if ($email_address ne $stripped_address) {
+					if (WARN("BAD_SIGN_OFF",
+					    "address part of email should not have comments: '$email_address'\n" . $herecurr) &&
+					    $fix) {
+						$fixed[$fixlinenr] =~ s/\Q$email_address\E/$stripped_address/;
+					}
+				}
+
+				# Only one name comment should be allowed
+				my $comment_count = () = $name_comment =~ /\([^\)]+\)/g;
+				if ($comment_count > 1) {
 					WARN("BAD_SIGN_OFF",
-					     "email address '$email' might be better as '$suggested_email'\n" . $herecurr);
+					     "Use a single name comment in email: '$email'\n" . $herecurr);
+				}
+
+
+				# stable@vger.kernel.org or stable@kernel.org shouldn't
+				# have an email name. In addition commments should strictly
+				# begin with a #
+				if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
+					if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
+					    ($email_name ne ""))) {
+						my $cur_name = $email_name;
+						my $new_comment = $comment;
+
+						$cur_name =~ s/[a-zA-Z\s\-\"]+//g;
+						$new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
+						$new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
+						$new_comment = " # $new_comment" if length($new_comment) > 0;
+						my $new_email = "$email_address$new_comment";
+
+						if (WARN("BAD_SIGN_OFF",
+						    "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&
+						    $fix) {
+							$fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
+						}
+					}
+				} else {
+					if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
+						if (WARN("BAD_SIGN_OFF",
+						    "Unexpected content after email: '$email'\n" . $herecurr) &&
+						    $fix) {
+							my $new_comment = $comment;
+							$new_comment =~ s/^(?:\/\*|\.|\,)//g;
+							$new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
+							$new_comment = " ($new_comment)" if length($new_comment) > 0;
+							$fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
+						}
+					}
 				}
 			}
 
-- 
2.27.0

[Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing

[Dwaipayan Ray]

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

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 email 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.
  Relaxation is given to comments beginning with #.
- Stable addresses should not begin with a name.
- Comments in stable addresses should begin only
  with a #.

Improvements to parsing:

- Detect and report unexpected content after email.
- Quoted names are excluded from comment parsing.
- Trailing dots, commas or quotes in email are removed during
  formatting. Correspondingly a BAD_SIGN_OFF warning
  is emitted.
- Improperly quoted email like '"name <address>"' are now
  warned about.

In addition, added fixes for all the possible rules.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 97 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 80 insertions(+), 17 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fab38b493cef..d866c4321182 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1152,6 +1152,7 @@ sub parse_email {
 	my ($formatted_email) = @_;
 
 	my $name = "";
+	my $quoted = "";
 	my $name_comment = "";
 	my $address = "";
 	my $comment = "";
@@ -1183,14 +1184,20 @@ sub parse_email {
 		}
 	}
 
-	$comment = trim($comment);
-	$name = trim($name);
-	$name =~ s/^\"|\"$//g;
-	if ($name =~ s/(\s*\([^\)]+\))\s*//) {
-		$name_comment = trim($1);
+	# Extract comments from names excluding quoted parts
+	# "John A. (Kennedy)" - Do not extract
+	if ($name =~ s/\"(.+)\"//) {
+		$quoted = $1;
+	}
+	while ($name =~ s/\s*($balanced_parens)\s*/ /) {
+		$name_comment .= trim($1);
 	}
+	$name =~ s/^[ \"]+|[ \"]+$//g;
+	$name = trim("$quoted $name");
+
 	$address = trim($address);
 	$address =~ s/^\<|\>$//g;
+	$comment = trim($comment);
 
 	if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
 		$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
@@ -1205,17 +1212,20 @@ sub format_email {
 
 	my $formatted_email;
 
-	$name_comment = trim($name_comment);
-	$comment = trim($comment);
-	$name = trim($name);
-	$name =~ s/^\"|\"$//g;
+	$name =~ s/^[ \"]+|[ \"]+$//g;
 	$address = trim($address);
+	$address =~ s/(?:\.|\,|\")+$//; ##trailing commas, dots or quotes
 
 	if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
 		$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
 		$name = "\"$name\"";
 	}
 
+	$name_comment = trim($name_comment);
+	$name_comment = " $name_comment" if length($name_comment) > 0;
+	$comment = trim($comment);
+	$comment = " $comment" if length($comment) > 0;
+
 	if ("$name" eq "") {
 		$formatted_email = "$address";
 	} else {
@@ -1233,15 +1243,11 @@ sub reformat_email {
 }
 
 sub same_email_addresses {
-	my ($email1, $email2, $match_comment) = @_;
+	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);
 
-	if ($match_comment != 1) {
-		return $email1_name eq $email2_name &&
-		       $email1_address eq $email2_address;
-	}
 	return $email1_name eq $email2_name &&
 	       $email1_address eq $email2_address &&
 	       $name1_comment eq $name2_comment &&
@@ -2704,7 +2710,7 @@ sub process {
 			$signoff++;
 			$in_commit_log = 0;
 			if ($author ne ''  && $authorsignoff != 1) {
-				if (same_email_addresses($1, $author, 1)) {
+				if (same_email_addresses($1, $author)) {
 					$authorsignoff = 1;
 				} else {
 					my $ctx = $1;
@@ -2800,9 +2806,66 @@ 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) {
+						$fixed[$fixlinenr] =~ s/\Q$email\E/$suggested_email/;
+					}
+				}
+
+				# Address part shouldn't have comments
+				my $stripped_address = $email_address;
+				$stripped_address =~ s/\([^\(\)]*\)//g;
+				if ($email_address ne $stripped_address) {
+					if (WARN("BAD_SIGN_OFF",
+					    "address part of email should not have comments: '$email_address'\n" . $herecurr) &&
+					    $fix) {
+						$fixed[$fixlinenr] =~ s/\Q$email_address\E/$stripped_address/;
+					}
+				}
+
+				# Only one name comment should be allowed
+				my $comment_count = () = $name_comment =~ /\([^\)]+\)/g;
+				if ($comment_count > 1) {
 					WARN("BAD_SIGN_OFF",
-					     "email address '$email' might be better as '$suggested_email'\n" . $herecurr);
+					     "Use a single name comment in email: '$email'\n" . $herecurr);
+				}
+
+
+				# stable@vger.kernel.org or stable@kernel.org shouldn't
+				# have an email name. In addition commments should strictly
+				# begin with a #
+				if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
+					if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
+					    ($email_name ne ""))) {
+						my $cur_name = $email_name;
+						my $new_comment = $comment;
+
+						$cur_name =~ s/[a-zA-Z\s\-\"]+//g;
+						$new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
+						$new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
+						$new_comment = " # $new_comment" if length($new_comment) > 0;
+						my $new_email = "$email_address$new_comment";
+
+						if (WARN("BAD_SIGN_OFF",
+						    "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&
+						    $fix) {
+							$fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
+						}
+					}
+				} else {
+					if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
+						if (WARN("BAD_SIGN_OFF",
+						    "Unexpected content after email: '$email'\n" . $herecurr) &&
+						    $fix) {
+							my $new_comment = $comment;
+							$new_comment =~ s/^(?:\/\*|\.|\,)//g;
+							$new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
+							$new_comment = " ($new_comment)" if length($new_comment) > 0;
+							$fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
+						}
+					}
 				}
 			}
 
-- 
2.27.0

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

Re: [PATCH v4] checkpatch: improve email parsing

[Joe Perches]

On Sat, 2020-11-07 at 03:15 +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.

Assuming it all works, this looks good.  I haven't tested it.

How did you test the $fix bits?

Trivial notes:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> +				# stable@vger.kernel.org or stable@kernel.org shouldn't
> +				# have an email name. In addition commments should strictly
> +				# begin with a #
> +				if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {

Likely better to test with a case insensitive match so
STABLE@vger.kernel.org and such are still warned.

				if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {

> +					if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> +					    ($email_name ne ""))) {

|| $sign_off !~ /^cc:/i ?

> +						my $cur_name = $email_name;
> +						my $new_comment = $comment;
> +
> +						$cur_name =~ s/[a-zA-Z\s\-\"]+//g;
> +						$new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
> +						$new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
> +						$new_comment = " # $new_comment" if length($new_comment) > 0;
> +						my $new_email = "$email_address$new_comment";
> +
> +						if (WARN("BAD_SIGN_OFF",
> +						    "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&

These message lines should be aligned to the next open parenthesis
(7 tabs, 1 space)

> +						    $fix) {
> +							$fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
> +						}

As a cc to stable isn't a sign-off, perhaps this should be a different
"BAD_<FOO>" like "BAD_STABLE_ADDRESS_STYLE" or such.

> +					}
> +				} else {

elsif ?

> +					if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
> +						if (WARN("BAD_SIGN_OFF",
> +						    "Unexpected content after email: '$email'\n" . $herecurr) &&

7 tabs, 1 space

> +						    $fix) {
> +							my $new_comment = $comment;
> +							$new_comment =~ s/^(?:\/\*|\.|\,)//g;
> +							$new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
> +							$new_comment = " ($new_comment)" if length($new_comment) > 0;
> +							$fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
> +						}
> +					}
>  				}
>  			}
>  
> 

Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing

[Joe Perches]

On Sat, 2020-11-07 at 03:15 +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.

Assuming it all works, this looks good.  I haven't tested it.

How did you test the $fix bits?

Trivial notes:

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> +				# stable@vger.kernel.org or stable@kernel.org shouldn't
> +				# have an email name. In addition commments should strictly
> +				# begin with a #
> +				if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {

Likely better to test with a case insensitive match so
STABLE@vger.kernel.org and such are still warned.

				if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {

> +					if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> +					    ($email_name ne ""))) {

|| $sign_off !~ /^cc:/i ?

> +						my $cur_name = $email_name;
> +						my $new_comment = $comment;
> +
> +						$cur_name =~ s/[a-zA-Z\s\-\"]+//g;
> +						$new_comment =~ s/^[\s\#\(\[]+|[\s\)\]]+$//g;
> +						$new_comment = trim("$new_comment $cur_name") if $cur_name ne $new_comment;
> +						$new_comment = " # $new_comment" if length($new_comment) > 0;
> +						my $new_email = "$email_address$new_comment";
> +
> +						if (WARN("BAD_SIGN_OFF",
> +						    "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) &&

These message lines should be aligned to the next open parenthesis
(7 tabs, 1 space)

> +						    $fix) {
> +							$fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/;
> +						}

As a cc to stable isn't a sign-off, perhaps this should be a different
"BAD_<FOO>" like "BAD_STABLE_ADDRESS_STYLE" or such.

> +					}
> +				} else {

elsif ?

> +					if ($comment ne "" && $comment !~ /^(?:#.+|\(.+\))$/) {
> +						if (WARN("BAD_SIGN_OFF",
> +						    "Unexpected content after email: '$email'\n" . $herecurr) &&

7 tabs, 1 space

> +						    $fix) {
> +							my $new_comment = $comment;
> +							$new_comment =~ s/^(?:\/\*|\.|\,)//g;
> +							$new_comment =~ s/^[\s\{\[]+|[\s\}\]]+$//g;
> +							$new_comment = " ($new_comment)" if length($new_comment) > 0;
> +							$fixed[$fixlinenr] =~ s/\s*\Q$comment\E$/$new_comment/;
> +						}
> +					}
>  				}
>  			}
>  
> 


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

Re: [PATCH v4] checkpatch: improve email parsing

[Dwaipayan Ray]

On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-11-07 at 03:15 +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.
>
> Assuming it all works, this looks good.  I haven't tested it.
>
> How did you test the $fix bits?
>
Hi,
I actually dumped about 17k unique emails from git log, put it in one of
my previous patches, and ran checkpatch with --fix on it.
I checked the diff and most of the cases looked pretty good to me.
I could send the diff output if you like?

> Trivial notes:
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > +                             # stable@vger.kernel.org or stable@kernel.org shouldn't
> > +                             # have an email name. In addition commments should strictly
> > +                             # begin with a #
> > +                             if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
>
> Likely better to test with a case insensitive match so
> STABLE@vger.kernel.org and such are still warned.

Sure, I will do that.
>
>                                 if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
>
> > +                                     if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > +                                         ($email_name ne ""))) {
>
> || $sign_off !~ /^cc:/i ?

I actually had a doubt about that one. Only the stable address with
Cc: should be checked right? Or something else?

What about those stable addresses with tags other than Cc: ? Should
a change be suggested?

Thanks,
Dwaipayan.

Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing

[Dwaipayan Ray]

On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2020-11-07 at 03:15 +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.
>
> Assuming it all works, this looks good.  I haven't tested it.
>
> How did you test the $fix bits?
>
Hi,
I actually dumped about 17k unique emails from git log, put it in one of
my previous patches, and ran checkpatch with --fix on it.
I checked the diff and most of the cases looked pretty good to me.
I could send the diff output if you like?

> Trivial notes:
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > +                             # stable@vger.kernel.org or stable@kernel.org shouldn't
> > +                             # have an email name. In addition commments should strictly
> > +                             # begin with a #
> > +                             if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
>
> Likely better to test with a case insensitive match so
> STABLE@vger.kernel.org and such are still warned.

Sure, I will do that.
>
>                                 if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
>
> > +                                     if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > +                                         ($email_name ne ""))) {
>
> || $sign_off !~ /^cc:/i ?

I actually had a doubt about that one. Only the stable address with
Cc: should be checked right? Or something else?

What about those stable addresses with tags other than Cc: ? Should
a change be suggested?

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

Re: [PATCH v4] checkpatch: improve email parsing

[Joe Perches]

On Sat, 2020-11-07 at 10:11 +0530, Dwaipayan Ray wrote:
> On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
> > 
> > On Sat, 2020-11-07 at 03:15 +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.
> > 
> > Assuming it all works, this looks good.  I haven't tested it.
> > 
> > How did you test the $fix bits?
> > 
> Hi,
> I actually dumped about 17k unique emails from git log, put it in one of
> my previous patches, and ran checkpatch with --fix on it.
> I checked the diff and most of the cases looked pretty good to me.
> I could send the diff output if you like?

Please.  Likely just to me as I imagine it's not interesting to most.
 
> > Trivial notes:
> > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > +                             # stable@vger.kernel.org or stable@kernel.org shouldn't
> > > +                             # have an email name. In addition commments should strictly
> > > +                             # begin with a #
> > > +                             if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
> > 
> > Likely better to test with a case insensitive match so
> > STABLE@vger.kernel.org and such are still warned.
> 
> Sure, I will do that.
> > 
> >                                 if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
> > 
> > > +                                     if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > > +                                         ($email_name ne ""))) {
> > 
> > > > $sign_off !~ /^cc:/i ?
> 
> I actually had a doubt about that one. Only the stable address with
> Cc: should be checked right? Or something else?

yes.
 
> What about those stable addresses with tags other than Cc: ? Should
> a change be suggested?

Ideally yes, but there were very few of those in the git commit
history so it's probably not a big deal one way or another.

Re: [Linux-kernel-mentees] [PATCH v4] checkpatch: improve email parsing

[Joe Perches]

On Sat, 2020-11-07 at 10:11 +0530, Dwaipayan Ray wrote:
> On Sat, Nov 7, 2020 at 3:34 AM Joe Perches <joe@perches.com> wrote:
> > 
> > On Sat, 2020-11-07 at 03:15 +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.
> > 
> > Assuming it all works, this looks good.  I haven't tested it.
> > 
> > How did you test the $fix bits?
> > 
> Hi,
> I actually dumped about 17k unique emails from git log, put it in one of
> my previous patches, and ran checkpatch with --fix on it.
> I checked the diff and most of the cases looked pretty good to me.
> I could send the diff output if you like?

Please.  Likely just to me as I imagine it's not interesting to most.
 
> > Trivial notes:
> > 
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > []
> > > +                             # stable@vger.kernel.org or stable@kernel.org shouldn't
> > > +                             # have an email name. In addition commments should strictly
> > > +                             # begin with a #
> > > +                             if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/) {
> > 
> > Likely better to test with a case insensitive match so
> > STABLE@vger.kernel.org and such are still warned.
> 
> Sure, I will do that.
> > 
> >                                 if ($email =~ /\bstable\@(?:vger\.)?kernel\.org\b/i) {
> > 
> > > +                                     if ($sign_off =~ /cc:$/i && (($comment ne "" && $comment !~ /^#.+/) ||
> > > +                                         ($email_name ne ""))) {
> > 
> > > > $sign_off !~ /^cc:/i ?
> 
> I actually had a doubt about that one. Only the stable address with
> Cc: should be checked right? Or something else?

yes.
 
> What about those stable addresses with tags other than Cc: ? Should
> a change be suggested?

Ideally yes, but there were very few of those in the git commit
history so it's probably not a big deal one way or another.


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