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

[Haavard Skinnemoen]

JerryVanBaren <gerald.vanbaren at ge.com> wrote:
> Georg Schardt wrote:
> > ---
> >  include/configs/FX12MM.h |   12 ++++++------
> >  1 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/configs/FX12MM.h b/include/configs/FX12MM.h
> > index b47e403..8b8d41c 100644
> > --- a/include/configs/FX12MM.h
> > +++ b/include/configs/FX12MM.h
> > @@ -15,28 +15,28 @@
> >  #define CONFIG_DOS_PARTITION    1
> >  #define CFG_SYSTEMACE_BASE      XPAR_SYSACE_0_BASEADDR
> >  #define CFG_SYSTEMACE_WIDTH     XPAR_SYSACE_0_MEM_WIDTH
> > -#define ADD_SYSTEMACE_CMDS      (| CFG_CMD_FAT)
> > +#define ADD_SYSTEMACE_CMDS      ( | CFG_CMD_FAT )
> >  #define RM_SYSTEMACE_CMDS
> >  #else
> >  #define ADD_SYSTEMACE_CMDS
> > -#define RM_SYSTEMACE_CMDS       | CFG_CMD_FAT
> > +#define RM_SYSTEMACE_CMDS       ( | CFG_CMD_FAT )
> 
> Dear List,
> 
> 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?

In this particular case, I'd recommend not starting the macro with a
binary operator in the first place. That's just _wrong_.

Better define ADD_SYSTEMACE_CMDS etc. as 0 if no flags are to be
added/removed, and add the operator at the callsite.

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

The checkpatch author probably never considered that anyone would
actually define a macro beginning with a binary operator, so it's no
surprise that it comes up with strange suggestions.

That said, putting parentheses around macro contents is a good rule in
general since it might avoid quite a few surprises. E.g.

#define FOO	-1

	bar = 2FOO;	/* should give a compile error, but doesn't */

> I'm serious about this question: in my day job I see a lot of mechanical 
> praying to the god of miserableC (MISRA-C) adding a HUGE amount of 
> unnecessary syntax noise such that it becomes hard to read the code 
> because of all the noise.  I've had people at work ask me why "we" 
> cannot write code that is as easy to understand as the linux kernel. 
> The answer is simple: "we" are slavishly and mechanically following the 
> god of "if it was good practice somewhere, sometime, it must always be a 
> good practice" and not applying good engineering judgment and experience.

I'm not particularly familiar with MISRA-C, but from what I've seen of
it, it's a particularly horrible example of following rules just for
the sake of the rules.

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

Agreed.

Haavard