Re: [Libguestfs] [PATCH nbdkit 1/2] common/include: Change unique-name macros to use __COUNTER__

On Tue, Feb 22, 2022 at 09:35:03PM +0000, Richard W.M. Jones wrote: > Previously the macros used __LINE__ which meant they created a unique > name specific to the line on which the macro was expanded. This > worked to a limited degree for cases like: > > #define FOO \ > ({ int NBDKIT_UNIQUE_NAME(foo) = 42; \ > NBDKIT_UNIQUE_NAME(foo) * 2 }) Missing a ; after '* 2'. > > since the “FOO” macro is usually expanded at one place so both uses of > NBDKIT_UNIQUE_NAME expanded to the same unique name. It didn't work > if FOO was used twice on the same line, eg: > > int i = FOO * FOO; This didn't actually fail. The failure we saw was more subtle: MIN (MIN (1, 2), 3) compiled (the inner MIN() creates an _x evaluated in the context of the initializer of the outer MIN's _x, which is not in scope until the initializer completes), but: MIN (1, MIN (2, 3)) failed to compile with -Werror -Wshadow, because now the inner MIN's _x is declared inside the scope of the initializer of the outer MIN's _y, when an outer _x is already in scope.

> > would fail, but this would work: > > int i = FOO * > FOO; Or, back to the example we actually hit, MIN (1, MIN (2, 3)) worked. > > Use __COUNTER__ instead of __LINE__, but NBDKIT_UNIQUE_NAME must now > be used differently. The FOO macro above must be rewritten as: > > #define FOO FOO_1(NBDKIT_UNIQUE_NAME(foo)) > #define FOO_1(foo) \ > ({ int foo = 42; \ > foo * 2 }) > > Thanks: Eric Blake > --- > +++ b/common/include/test-checked-overflow.c > @@ -39,29 +39,25 @@ > > #define TEST_MUL(a, b, result, expected_overflow, expected_result) \ > do { \ > - bool NBDKIT_UNIQUE_NAME(_actual_overflow); \ > + bool actual_overflow; \ > \ > - NBDKIT_UNIQUE_NAME(_actual_overflow) = \ > - MUL_OVERFLOW_FALLBACK ((a), (b), (result)); \ > - assert (NBDKIT_UNIQUE_NAME(_actual_overflow) == (expected_overflow)); \ > + actual_overflow = MUL_OVERFLOW_FALLBACK ((a), (b), (result)); \ Extra spacing after = The commit message may need touching up, but ACK to the change itself.