Re: [PATCH 7/7] qobject atomics osdep: Make a few macros more hygienic

[Markus Armbruster <armbru@xxxxxxxxxx>]

Cédric Le Goater <clg@xxxxxxxx> writes:

> On 8/31/23 16:30, Eric Blake wrote:
>> On Thu, Aug 31, 2023 at 03:25:46PM +0200, Markus Armbruster wrote:
>> [This paragraph written last: Bear with my stream of consciousness
>> review below, where I end up duplicating some of the conslusions you
>> reached before the point where I saw where the patch was headed]
>> 
>>> Variables declared in macros can shadow other variables.  Much of the
>>> time, this is harmless, e.g.:
>>>
>>>      #define _FDT(exp)                                                  \
>>>          do {                                                           \
>>>              int ret = (exp);                                           \
>>>              if (ret < 0) {                                             \
>>>                  error_report("error creating device tree: %s: %s",   \
>>>                          #exp, fdt_strerror(ret));                      \
>>>                  exit(1);                                               \
>>>              }                                                          \
>>>          } while (0)
>> Which is why I've seen some projects require a strict namespace
>> separation: if all macro parameters and any identifiers declared in
>> macros use either a leading or a trailing _ (I prefer a trailing one,
>> to avoid risking conflicts with libc reserved namespace; but leading
>> is usually okay), and all other identifiers avoid that namespace, then
>> you will never have shadowing by calling a macro from normal code.
>
> I started fixing the _FDT() macro since it is quite noisy at compile.
> Same for qemu_fdt_setprop_cells(). So are we ok with names like 'ret_'
> and 'i_' ? I used a 'local_' prefix for now but I can change.

I believe identifiers with a '_' suffix are just fine in macros.  We
have quite a few of them already.

> I also have a bunch of fixes for ppc.

Appreciated!