From: Tilman Schmidt <tilman@imap.cc>
To: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Tilman Schmidt <tilman@imap.cc>,
Andy Whitcroft <apw@canonical.com>,
LKML <linux-kernel@vger.kernel.org>,
Andy Walls <awalls@radix.net>
Subject: Re: checkpatch is not detecting bad whitespacing when using macros on formulas
Date: Mon, 12 Oct 2009 00:04:33 +0200 [thread overview]
Message-ID: <4AD25671.7050303@imap.cc> (raw)
In-Reply-To: <20091011112624.39830ef9@pedra.chehab.org>
[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]
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@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 254 bytes --]
next prev parent reply other threads:[~2009-10-11 22:05 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-10-11 10:19 checkpatch is not detecting bad whitespacing when using macros on formulas Mauro Carvalho Chehab
2009-10-11 11:01 ` Tilman Schmidt
2009-10-11 14:26 ` Mauro Carvalho Chehab
2009-10-11 22:04 ` Tilman Schmidt [this message]
2009-10-26 11:20 ` Andy Whitcroft
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4AD25671.7050303@imap.cc \
--to=tilman@imap.cc \
--cc=apw@canonical.com \
--cc=awalls@radix.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mchehab@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox