linux-kernel - Re: checkpatch is not detecting bad whitespacing when using macros on formulas

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: <4AD25671.7050303@imap.cc>
Date:	Mon, 12 Oct 2009 00:04:33 +0200
From:	Tilman Schmidt <tilman@...p.cc>
To:	Mauro Carvalho Chehab <mchehab@...radead.org>
CC:	Tilman Schmidt <tilman@...p.cc>,
	Andy Whitcroft <apw@...onical.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andy Walls <awalls@...ix.net>
Subject: Re: checkpatch is not detecting bad whitespacing when using macros
 on formulas

Mauro Carvalho Chehab schrieb:

> Chapter 3.1 of CodingStyle says:
> 
> 	Use one space around (on each side of) most binary and ternary operators,
> 	such as any of these:
> 
> 	        =  +  -  <  >  *  /  %  |  &  ^  <=  >=  ==  !=  ?  :
> 
> At the above, it should have an space before and after the first division
> operator.

Agreed.

> So, we should or fix checkpatch.pl or remove the above rule from
> CodingStyle.

No! There is no need (and indeed no way) for checkpatch.pl and
CodingStyle to agree 100 percent.

It's perfectly fine to ask a patch author to fix coding style
issues even if checkpatch.pl didn't complain about them. I've
yet to read of a patch author retorting that "checkpatch.pl
accepted it so it must be fine".

OTOH it's not fine at all if checkpatch.pl complains about
something that is not in fact forbidden by CodingStyle. It
gives patch authors a really hard time, because the standard
argument on LKML has become "checkpatch.pl complained about it
so it must be bad". Try to argue against that once and you'll
see what I mean.

So checkpatch.pl should strive, about all, not to produce
false positives, ie. not to complain about code that is in
fact fine. False negatives, ie. failing to complain about code
that is in violation of CodingStyle in some way, is much less
serious.

> Note that the patch has other similar troubles, like:
> 
> +       if (d > RXCLK_RCD+1)
> +                                CX23888_IR_REFCLK_FREQ/1000000);
> +       if (rem >= CX23888_IR_REFCLK_FREQ/1000000/2)
> +       clocks = CX23888_IR_REFCLK_FREQ/1000000 * (u64) ns; /* millicycles    */
> +       return DIV_ROUND_CLOSEST((n+1) * 100, 16);

Sure. But again, that's no reason to make checkpatch.pl complain
even more. It's enough of a pain as it is.

-- 
Tilman Schmidt                    E-Mail: tilman@...p.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)

Download attachment "signature.asc" of type "application/pgp-signature" (255 bytes)