* Re: [PATCH v3] checkpatch: improve email parsing
[not found] <20201105115949.39474-1-dwaipayanray1@gmail.com>
@ 2020-11-05 17:41 ` Joe Perches
2020-11-05 20:08 ` Greg KH
0 siblings, 1 reply; 2+ messages in thread
From: Joe Perches @ 2020-11-05 17:41 UTC (permalink / raw)
To: Dwaipayan Ray, stable, Greg KH
Cc: linux-kernel-mentees, linux-kernel, lukas.bulwahn, yashsri421
(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.
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v3] checkpatch: improve email parsing
2020-11-05 17:41 ` [PATCH v3] checkpatch: improve email parsing Joe Perches
@ 2020-11-05 20:08 ` Greg KH
0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2020-11-05 20:08 UTC (permalink / raw)
To: Joe Perches
Cc: Dwaipayan Ray, stable, linux-kernel-mentees, linux-kernel,
lukas.bulwahn, yashsri421
On Thu, Nov 05, 2020 at 09:41:15AM -0800, Joe Perches wrote:
> (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
That is what is documented it should be, yes.
> then
> stable@vger.kernel.org [ version info ]
Really? Ick, no wonder my email parsing scripts choke on that :)
> with some other relatively infrequently used outlier styles, some
> that use parentheses, but this is not frequent.
The one with '#' should be preferred. If not, we need to change our
documentation.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-05 20:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20201105115949.39474-1-dwaipayanray1@gmail.com>
2020-11-05 17:41 ` [PATCH v3] checkpatch: improve email parsing Joe Perches
2020-11-05 20:08 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).