Re: [PATCH] update checkpatch.pl to version 0.08

Randy Dunlap
2007-07-24T00:06:56Z

On Mon, 23 Jul 2007 16:08:26 -0700 Kok, Auke wrote:

> Andy Whitcroft wrote:
> > This version brings a number of new checks, and a number of bug
> > fixes. Of note:
> >
> > - warnings for multiple assignments per line
>
>
> This is bugged. e.g. the following line will hit this exception check:
>
> int i = some_function(a, b, c);

Agreed.

...

> This is a royal pain, since it now throws an ERROR for the obviously preferable
> piece of code below:
>
> if (err) {
> do_something();
> return -ERR;
> } else {
> do_somthing_else();
> }
>
>
>
> Also, CondingStyle explicitly permits this style (even encourages it):

Yes, Linus has recently written that he prefers that style also.

The problem (to me at least) is that this patch:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=e659ba4a0d2d471c0d73590f78e1a1b5a1eede48

was not discussed before it was merged. It was discussed a little bit
after it was merged. Basically we have no concensus on this patch,
just a dictate (which we can all live with).

> ---
> Do not unnecessarily use braces where a single statement will do.
>
> if (condition)
> action();
>
> This does not apply if one branch of a conditional statement is a single
> statement. Use braces in both branches.
>
> if (condition) {
> do_this();
> do_that();
> } else {
> otherwise();
> }
> ---
>
> So, IMO this test needs to go, unless the script becomes smart enough to know
> that either side of the else requires braces. It's definately not an ERROR.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***