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
| ||
|
Date: Sun, 9 Apr 2017 10:55:31 +0200 From: Andreas Mohr <andi@...as.de> To: Joe Perches <joe@...ches.com> Cc: Alfonso Lima <alfonsolimaastor@...il.com>, apw@...onical.com, linux-kernel@...r.kernel.org Subject: Re: [bug report] checkpatch: if statement does not need to be enclosed in parentheses On Sat, Apr 08, 2017 at 10:33:19AM -0700, Joe Perches wrote: > On Sat, 2017-04-08 at 17:07 +0100, Alfonso Lima wrote: > > Hi, > > > > In current linux-next, if you run the script on this piece of > > code: > > > > #define FOO(a) \ > > if (a) { \ > > something(); \ > > something_else(); \ > > } > > > > You should get: > > > > ERROR: Macros with complex values should be enclosed in parentheses > > #1: FILE: foo.c:1: > > +#define FOO(a) \ > > + if (a) { \ > > + something(); \ > > + something_else(); \ > > + } > > > > We could silence checkpatch.pl using "do {} while ()" around the > > if statement. However, the "if () {}" statement should be > > enough. If someone could confirm this, I'll go and fix it. > > Multiple if/else use is the reason do {} while (0) is suggested. > > if (bar()) > FOO(a); > else > baz(b); > So, if I read this right checkpatch.pl is generating a correct complaint in this case after all? However, then we seem to have the following issues remaining: - "Macros with complex values should be enclosed in parentheses" != "do {} while (0) is suggested" --> this is an imprecision which may cause confusion - *further* cases of this issue occurring may just as well suggest again "fixing" (i.e., "damaging") checkpatch.pl rather than simply obeying (following through on) its correct diagnosis --> checkpatch.pl behaviour is confusing enough that it may prompt incorrect core handling "repair actions", and this danger will remain permanently until checkpatch.pl reporting is improved to be less confusing/insufficient about its diagnosis Or perhaps this imprecision here again simply is due to the very frequently observable case of saying "thou shan't do this" without then also giving some tried-and-true *reason(s)* for such advice. ("ERROR: Macros with complex values should be enclosed in parentheses") HTH, Andreas Mohr
Powered by blists - more mailing lists