Re: [PATCH] checkpatch: add warning for non-lore mailing list URLs
From: Kees Cook
Date: Wed Feb 10 2021 - 03:23:36 EST
On Thu, Dec 17, 2020 at 04:50:41PM -0800, Joe Perches wrote:
> On Thu, 2020-12-17 at 17:56 -0600, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> >
> > The lkml.org, marc.info, spinics.net, etc archives are not quite as useful
> > as lore.kernel.org because they use different styles, add advertising, and
> > may disappear in the future. The lore archives are more consistent and
> > more likely to stick around, so prefer https://lore.kernel.org URLs when
> > they exist.
>
> Hi Bjorn.
>
> I like the idea, thanks, but a couple notes.
>
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
> > @@ -564,6 +564,17 @@ sub find_standard_signature {
> > return "";
> > }
>
> > +our $obsolete_archives = qr{(?xi:
> > + freedesktop.org/archives/dri-devel|
> > + lists.infradead.org|
> > + lkml.org|
> > + mail-archive.com|
> > + mailman.alsa-project.org/pipermail|
> > + marc.info|
> > + ozlabs.org/pipermail|
> > + spinics.net
> > +)};
>
> Strictly, these all need \Q \E escaping so uses like lkmlAorg do not match.
>
>
> > @@ -3101,6 +3112,12 @@ sub process {
> > }
> > }
> >
> > +# Check for mailing list archives other than lore.kernel.org
> > + if ($line =~ /(http|https):\/\/\S*$obsolete_archives/) {
>
> The https?:// doesn't seem necessary. Perhaps:
>
> if ($line =~ m{\b$obsolete_archives}) {
>
> > + WARN("PREFER_LORE_ARCHIVE",
> > + "Use lore.kernel.org archive links when possible; see https://lore.kernel.org/lists.html\n" . $herecurr);
>
> Perhaps:
> "Prefer lore.kernel.org links. see: https://www.kernel.org/lore.html#linking-to-list-discussions-from-commits\n" . $herecurr);
>
> So maybe instead:
> ---
> scripts/checkpatch.pl | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 00085308ed9d..c2a324d628a6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -564,6 +564,17 @@ sub find_standard_signature {
> return "";
> }
>
> +our $obsolete_archives = qr{(?xi:
> + \Qfreedesktop.org/archives/dri-devel\E |
> + \Qlists.infradead.org\E |
> + \Qlkml.org\E |
> + \Qmail-archive.com\E |
> + \Qmailman.alsa-project.org/pipermail\E |
> + \Qmarc.info\E |
> + \Qozlabs.org/pipermail\E |
> + \Qspinics.net\E
> +)};
> +
> our @typeListMisordered = (
> qr{char\s+(?:un)?signed},
> qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -3101,6 +3112,12 @@ sub process {
> }
> }
>
> + # Check for mailing list archives other than lore.kernel.org
> + if ($rawline =~ m{\b$obsolete_archives}) {
> + WARN("PREFER_LORE_ARCHIVE",
> + "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr);
> + }
> +
> # Check for added, moved or deleted files
> if (!$reported_maintainer_file && !$in_commit_log &&
> ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||
>
>
Ah, nice. Yes, this would be great to get added. Joe, can you respin as
a full path? Please consider it:
Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
--
Kees Cook