Re: [RFC PATCH 12/13] blk-mq.h: Fix parentheses around macro parameter use

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-05-05 14:40, Linus Torvalds wrote:
On Fri, May 5, 2023 at 6:56 AM Mathieu Desnoyers
<mathieu.desnoyers@xxxxxxxxxxxx> wrote:

Which way do we want to go with respect to the rvalue of the assignment
operator "=" in a macro ? (with or without parentheses)

In short:

#define m(x) do { z = (x); } while (0)

or

#define m(x) do { z = x; } while (0)

I suspect that the first one is preferred, just as a "don't even have
to think about it" thing.

In general, despite my suggestion of maybe using upper-case to show
odd syntax (and I may have suggested it, but I really don't like how
it looks, so I'm not at all convinced it's a good idea), to a
first-order approximation the rule should be:

  - always use parentheses around macros

  - EXCEPT:
      - when used purely as arguments to functions or other macros
      - when there is some syntax reason why it's not ok to add parens

I would add to this list of exceptions cases where the argument is used as an expression within brackets, e.g.

#define m(x) myvar[x]

Because the content within the brackets is already an expression.

The other exception I would add is when a parameter is used as an lvalue, as:

#define m(x) do { x = 2; } while (0)

because I cannot find a case where it would cause unexpected operator precedence.

Are you OK with those 2 additional exceptions ?


The "arguments to functions/macros" is because the comma separator
between arguments isn't even a operator (ie it is *not* a
comma-expression, it's multiple expressions separated by commas).
There is no "operator precedence" subtlety.

Good point.


So we have a lot of macros that are just wrappers around functions (or
other macros), and in that situation you do *not* then add more
parentheses, and doing something like

     #define update_screen(x) redraw_screen(x, 0)

is fine, and might even be preferred syntax because putting
parentheses around 'x' not only doesn't buy you anything, but just
makes things uglier.

And the "syntax reasons" can be due to the usual things: we not only
have that 'pass member name around' issue, but we have things like
string expansion etc, where adding parentheses anywhere to things like

     #define __stringify_1(x...)     #x
     #define __stringify(x...)       __stringify_1(x)

would obviously simply not work (or look at our "SYSCALL_DEFINEx()"
games for more complex examples with many layers of token pasting
etc).

But in general I would suggest against "this is the lowest priority
operator" kind of games. Nobody remembers the exact operator
precedence so well that they don't have to think about it.

So for example, we have

     #define scr_writew(val, addr) (*(addr) = (val))

to pick another VT example, and I think that's right both for 'addr'
(that requires the parentheses) and for 'val' (that might not require
it, but let's not make people think about it).

Indeed, brain power and reviewer time is a scarce resource. It's a shame to waste it on figuring out operator priority again and again.

Thanks,

Mathieu


--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux