Re: checkpatch is not detecting bad whitespacing when using macroson formulas

From: Tilman Schmidt
Date: Sun Oct 11 2009 - 18:06:39 EST


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@xxxxxxx
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)

Attachment: signature.asc
Description: OpenPGP digital signature