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

[Randy Dunlap <rdunlap@xenotime.net>]

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 ***