linux-kernel - Re: [PATCH v3] checkpatch: add a rule to check general block comment style

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: <alpine.DEB.2.22.394.2107191054260.9400@anisinha-lenovo>
Date:   Mon, 19 Jul 2021 10:58:30 +0530 (IST)
From:   Ani Sinha <ani@...sinha.ca>
To:     Joe Perches <joe@...ches.com>
cc:     Ani Sinha <ani@...sinha.ca>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        anirban.sinha@...ia.com, mikelley@...rosoft.com,
        Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>
Subject: Re: [PATCH v3] checkpatch: add a rule to check general block comment
 style



On Sun, 18 Jul 2021, Joe Perches wrote:

> On Sun, 2021-07-18 at 19:08 +0530, Ani Sinha wrote:
> > On Sun, 18 Jul 2021, Lukas Bulwahn wrote:
> > > On Fri, Jul 16, 2021 at 6:15 PM Ani Sinha <ani@...sinha.ca> wrote:
> > > > checkpatch maintainers, any comments?
> > > > On Wed, 14 Jul 2021, Ani Sinha wrote:
> > > > > The preferred style for long (multi-line) comments is:
> > > > >
> > > > > .. code-block:: c
> > > > >
> > > > >       /*
> > > > >        * This is the preferred style for multi-line
> > > > >        * comments in the Linux kernel source code.
> > > > >        * Please use it consistently.
> > > > >        *
> > > > >        * Description:  A column of asterisks on the left side,
> > > > >        * with beginning and ending almost-blank lines.
> > > > >        */
> > > > >
> > > > > It seems rule in checkpatch.pl is missing to ensure this for
> > > > > non-networking related changes. This patch adds this rule.
> []
> > > Honest feedback: IMHO, your commit message is unreadable and incomprehensible.
> >
> > OK. However, I fail to see how your above comment is useful without any
> > suggestion as to how to improve the commit log. I find having some test
> > data with the commit message valuable so that there is some sort of record
> > as to how the change was tested and with what arguments. Beyond that this
> > is not something I am really worried about. The commit message can be
> > modified and improved in any way reviewers like.
> >
> > >
> > > Now to the feature you are proposing:
> > >
> > > I do not think that it is good if checkpatch would point out a quite
> > > trivial syntactic issue that probably is currently violated many times
> > > (>10,000 or even >100,000 times?) in the overall repository. That will
> > > make checkpatch warn on many commits with this check and divert the
> > > attention from other checks that are more important than the style of
> > > starting comments.
> >
> > I have some strong opinions on this. Just because a rule has been violated
> > in the past does not mean it can continue to be violated in the future.
>
> Intensity of opinion varies considerably here.
>
> > > Further, some evaluation by Aditya shows that the distinction between
> > > NETWORKING COMMENT STYLE and GENERAL KERNEL COMMENT STYLE is not as
> > > easily split as currently encoded in the checkpatch script,
> > > https://lore.kernel.org/linux-kernel-mentees/cfff5784-9ca3-07f8-c51c-f1c82b2871e3@gmail.com/.
> > > So, this checkpatch check is largely wrong already as of now and most
> > > probably ignored by many contributors.
>
> The only reason the rule exists at all is because the networking maintainer
> was constantly telling people to change the comment style in patches.
>
> I don't care one way or another.
>
> // comments are fine
> /* comments are fine */
>
> In networking, multiline comments are almost exclusively like
> what Linus himself does not like:
>
> 	/* comment
> 	 * ...
> 	 */
>
> but in other subsystems, the styles of multiline comments varies.
>
> Either works, there is no single standard.
>

OK then in that case, maybe update
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v5.14-rc2#n584

It is confusing to patch submitters (and it happened to me with a recent
patch) that the reviewer insists on a particular commenting style when
checkpatch does not enforce it. Its confusing for reviewers too.


> And as the referenced link by Aditya somewhat shows, the nominal
> rule compliance varies by the age of the code.  No one care much
> about code submitted a couple decades ago for subsystems and drivers
> that are effectively obsolete...
>
>
>