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]

Date:   Sun, 18 Jul 2021 17:18:07 -0700
From:   Joe Perches <joe@...ches.com>
To:     Ani Sinha <ani@...sinha.ca>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>
Cc:     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, 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.

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...