linux-kernel - Re: [PATCH v2] checkpatch: improve email parsing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABJPP5D3v9ZGCCqn_wg43H5CN0ff=C8J8jDGYU6LHJBBn6-wAw@mail.gmail.com>
Date:   Tue, 3 Nov 2020 13:32:38 +0530
From:   Dwaipayan Ray <dwaipayanray1@...il.com>
To:     Joe Perches <joe@...ches.com>
Cc:     linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Aditya Srivastava <yashsri421@...il.com>
Subject: Re: [PATCH v2] checkpatch: improve email parsing

On Tue, Nov 3, 2020 at 12:58 PM Joe Perches <joe@...ches.com> wrote:
>
> On Tue, 2020-11-03 at 11:28 +0530, Dwaipayan Ray wrote:
> > On Tue, Nov 3, 2020 at 11:18 AM Dwaipayan Ray <dwaipayanray1@...il.com> wrote:
> > >
> > > 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@...r.kernel.org> # X.X
> > >
> > > Improve email parsing mechanism in checkpatch.
> > >
> > > What is added:
> > >
> > > - Support for multiple name/address comments.
> > > - Improved handling of quoted names.
> > > - Sanitize improperly formatted comments.
> > > - Sanitize trailing semicolon or dot after email.
> []
> > What do you think? Should warnings for the names which should
> > be quoted be reported considering this result?
>
> Clearly the quote suggestion is unnecessary.
>
> I think that "cc: stable@(?:vger\.)?kernel\.org" should be
> treated differently from other forms of invalid/odd address lines.
>
> My suggestion is that the case insensitive form of
>
> Cc: stable@...r.kernel.org
>
> or only another similar case insensitive forms with a
> # comment separator like
>
> Cc: <stable@...r.kernel.org> # some comment
>
> be acceptable for stable.
>
> All other forms with stable@ should emit some message.
>
> And other <foo>-by: and cc: addresses should only have a form like
>
> Signed-off-by: "Full.Name" (possible comment) <email@...ain.tld>
> or
> Signed-off-by: Full Name (possible comment) <email@...ain.tld>
>
> etc..
>
> and any additional content after .tld in the email address be flagged
> with some message like "unexpected content after email address" rather
> than "might be better as".
>
> What do you think best?
>

Hello,
I think the warning "unexpected content after email" is more reasonable.
But maybe we can allow addresses of type:

Full Name <email@...ain.tld> (comment here)
or
Full Name <email@...ain.tld> [another comment]

I checked the following:
$ git log --format=email -100000 | grep -P '^(?:[\w\-]+-by:|cc:|CC:|Cc:)' \
| grep -P ".*<\S+\@\S+>\s*(?:\(|\[|\/)" | wc -l
280

And for stable ones, I found another pattern which is common:
Cc: <stable@...r.kernel.org> [X.X]
and only few instances of:
cc: stable@...r.kernel.org (X.X)

Apart from these no other style has been used for stable so far.
Maybe placing it in the generic check would suffice.

So I think we can introduce the unexpected content warning for
anything other than the braces or c89 comment styles. And
maybe a separate check for stable won't be necessary then.

What is your thought on this?

Thanks,
Dwaipayan.