JerryVanBaren gerald.vanbaren@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