Messages in this thread | | | Date | Mon, 23 Jul 2007 17:11:32 -0700 | From | Randy Dunlap <> | Subject | Re: [PATCH] update checkpatch.pl to version 0.08 |
| |
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 *** - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |