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

On 02/23/22 00:05, Eric Blake wrote: > 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. I wish we could suppress -Wshadow in macro definitions, but I think "#pragma GCC diagnostics" may not work that way (or is not available on all the necessary compilers) :/

for the series Acked-by: Laszlo Ersek <lersek(a)redhat.com&gt; Laszlo > >> >> 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. >