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

[Cédric Le Goater]

On 9/1/23 16:50, Markus Armbruster wrote:
> 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.

ok

> > I also have a bunch of fixes for ppc.
>
> Appreciated!

count me on for the ppc files :

 hw/ppc/pnv_psi.c
 hw/ppc/spapr.c
 hw/ppc/spapr_drc.c
 hw/ppc/spapr_pci.c
 include/hw/ppc/fdt.h

and surely some other files under target/ppc/

This one was taken care of by Phil:

 include/sysemu/device_tree.h

C.