On 9/1/23 16:50, Markus Armbruster wrote:
Cédric Le Goater <c...@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.