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

[Richard W.M. Jones]

On Wed, Feb 23, 2022 at 10:28:17AM +0100, Laszlo Ersek wrote:
> 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) :/

I tried that (using _Pragma) but couldn't make it work.  It hits this
nest of bugs in GCC:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78657

I think the current solution is better in the end, so all good.

Rich.

> for the series
> 
> Acked-by: Laszlo Ersek <lersek at redhat.com>
> 
> 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.
> > 

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top