[U-Boot] [PATCH 2/3] fix checkpatch errors

Wolfgang Denk wd at denx.de
Mon Sep 8 14:38:24 CEST 2008


Dear JerryVanBaren,

In message <48C51773.5020803 at ge.com> you wrote:
> Georg Schardt wrote:
...
> > -#define RM_SYSTEMACE_CMDS       | CFG_CMD_FAT
> > +#define RM_SYSTEMACE_CMDS       ( | CFG_CMD_FAT )
...

> Philosophical question: is it better to put silly parenthesis around 
> #defines to make checkpatch shut up or to accept that checkpatch isn't 
> perfect and let it bitch about things that were done intentionally and 
> make sense per their usage?

Well, the most important thin is actually that the code is correct,
and the second important thing is that it can be compiled.

With above definitions of "( | CFG_CMD_FAT )" and  similar,  I  think
that  the code would NOT compile; instead, I expect GCC to issue some
"error: expected expression before '|' token"  error  messages  (most
probably  followed by some other "error: called object 'foo' is not a
function" erros.

> (Yes, I see the first one already had () and the change is just fixing 
> the spacing.)

No matter if it fixes spacing or not - it breaks the code.


> I'm serious about this question: in my day job I see a lot of mechanical 

OK, here is a serious anser: no. Such changes as suggested above make
zero sense even if they don't effect correctness of code.

See my previous discussion with Guennadi - changing all "return (1);"
into "return 1;" makes no sense to me when it's done just to silence
checkpatch.pl.

> It is WRONG to let our tools rule us.[1]

Agreed 100%.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Do not underestimate the value of print statements for debugging.


More information about the U-Boot mailing list