[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 7/7] qobject atomics osdep: Make a few macros more hygienic
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 7/7] qobject atomics osdep: Make a few macros more hygienic |
Date: |
Fri, 01 Sep 2023 16:50:02 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Cédric Le Goater <clg@kaod.org> 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!