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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 23 Jul 2007 17:11:32 -0700
From:	Randy Dunlap <rdunlap@...otime.net>
To:	"Kok, Auke" <auke-jan.h.kok@...el.com>
Cc:	Andy Whitcroft <apw@...dowen.org>, Andrew Morton <akpm@...l.org>,
	Randy Dunlap <rdunlap@...otime.net>,
	Joel Schopp <jschopp@...tin.ibm.com>,
	linux-kernel@...r.kernel.org
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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/