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 }) 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; would fail, but this would work: int i = FOO * FOO; 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 --- common/include/checked-overflow.h | 36 ++++++++++++++++---------- common/include/test-checked-overflow.c | 24 +++++++---------- common/include/unique-name.h | 10 ++++--- common/utils/cleanup.h | 26 ++++++++++++------- 4 files changed, 54 insertions(+), 42 deletions(-) diff --git a/common/include/checked-overflow.h b/common/include/checked-overflow.h index 4f81d4c1..546c930b 100644 --- a/common/include/checked-overflow.h +++ b/common/include/checked-overflow.h @@ -131,33 +131,41 @@ * that the test suite can always test the fallback. */ #define ADD_OVERFLOW_FALLBACK(a, b, r) \ + ADD_OVERFLOW_FALLBACK_1 ((a), (b), (r), \ + NBDKIT_UNIQUE_NAME (_overflow), \ + NBDKIT_UNIQUE_NAME (_tmp)) +#define ADD_OVERFLOW_FALLBACK_1(a, b, r, overflow, tmp) \ ({ \ - bool NBDKIT_UNIQUE_NAME(_overflow); \ - uintmax_t NBDKIT_UNIQUE_NAME(_tmp); \ + bool overflow; \ + uintmax_t tmp; \ \ STATIC_ASSERT_UNSIGNED_INT (a); \ STATIC_ASSERT_UNSIGNED_INT (b); \ STATIC_ASSERT_UNSIGNED_INT (*(r)); \ - NBDKIT_UNIQUE_NAME(_overflow) = check_add_overflow ((a), (b), \ - (typeof (*(r)))-1, \ - &NBDKIT_UNIQUE_NAME(_tmp)); \ - *(r) = NBDKIT_UNIQUE_NAME(_tmp); \ - NBDKIT_UNIQUE_NAME(_overflow); \ + overflow = check_add_overflow ((a), (b), \ + (typeof (*(r)))-1, \ + &tmp); \ + *(r) = tmp; \ + overflow; \ }) #define MUL_OVERFLOW_FALLBACK(a, b, r) \ + MUL_OVERFLOW_FALLBACK_1 ((a), (b), (r), \ + NBDKIT_UNIQUE_NAME (_overflow), \ + NBDKIT_UNIQUE_NAME (_tmp)) +#define MUL_OVERFLOW_FALLBACK_1(a, b, r, overflow, tmp) \ ({ \ - bool NBDKIT_UNIQUE_NAME(_overflow); \ - uintmax_t NBDKIT_UNIQUE_NAME(_tmp); \ + bool overflow; \ + uintmax_t tmp; \ \ STATIC_ASSERT_UNSIGNED_INT (a); \ STATIC_ASSERT_UNSIGNED_INT (b); \ STATIC_ASSERT_UNSIGNED_INT (*(r)); \ - NBDKIT_UNIQUE_NAME(_overflow) = check_mul_overflow ((a), (b), \ - (typeof (*(r)))-1, \ - &NBDKIT_UNIQUE_NAME(_tmp)); \ - *(r) = NBDKIT_UNIQUE_NAME(_tmp); \ - NBDKIT_UNIQUE_NAME(_overflow); \ + overflow = check_mul_overflow ((a), (b), \ + (typeof (*(r)))-1, \ + &tmp); \ + *(r) = tmp; \ + overflow; \ }) /* Assert, at compile time, that the expression "x" has some unsigned integer diff --git a/common/include/test-checked-overflow.c b/common/include/test-checked-overflow.c index 402ec98b..fa0c974f 100644 --- a/common/include/test-checked-overflow.c +++ b/common/include/test-checked-overflow.c @@ -39,29 +39,25 @@ #define TEST_ADD(a, b, result, expected_overflow, expected_result) \ do { \ - bool NBDKIT_UNIQUE_NAME(_actual_overflow); \ + bool actual_overflow; \ \ - NBDKIT_UNIQUE_NAME(_actual_overflow) = \ - ADD_OVERFLOW_FALLBACK ((a), (b), (result)); \ - assert (NBDKIT_UNIQUE_NAME(_actual_overflow) == (expected_overflow)); \ + actual_overflow = ADD_OVERFLOW_FALLBACK ((a), (b), (result)); \ + assert (actual_overflow == (expected_overflow)); \ assert (*(result) == (expected_result)); \ - NBDKIT_UNIQUE_NAME(_actual_overflow) = \ - ADD_OVERFLOW_FALLBACK ((b), (a), (result)); \ - assert (NBDKIT_UNIQUE_NAME(_actual_overflow) == (expected_overflow)); \ + actual_overflow = ADD_OVERFLOW_FALLBACK ((b), (a), (result)); \ + assert (actual_overflow == (expected_overflow)); \ assert (*(result) == (expected_result)); \ } while (0) #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)); \ + assert (actual_overflow == (expected_overflow)); \ assert (*(result) == (expected_result)); \ - NBDKIT_UNIQUE_NAME(_actual_overflow) = \ - MUL_OVERFLOW_FALLBACK ((b), (a), (result)); \ - assert (NBDKIT_UNIQUE_NAME(_actual_overflow) == (expected_overflow)); \ + actual_overflow = MUL_OVERFLOW_FALLBACK ((b), (a), (result)); \ + assert (actual_overflow == (expected_overflow)); \ assert (*(result) == (expected_result)); \ } while (0) diff --git a/common/include/unique-name.h b/common/include/unique-name.h index ff5d67db..2cd86d49 100644 --- a/common/include/unique-name.h +++ b/common/include/unique-name.h @@ -33,9 +33,11 @@ #ifndef NBDKIT_UNIQUE_NAME_H #define NBDKIT_UNIQUE_NAME_H -/* https://stackoverflow.com/a/1597129 */ -#define XXUNIQUE_NAME(name, line) name ## line -#define XUNIQUE_NAME(name, line) XXUNIQUE_NAME (name, line) -#define NBDKIT_UNIQUE_NAME(name) XUNIQUE_NAME (name, __LINE__) +/* https://stackoverflow.com/a/1597129 + * https://stackoverflow.com/a/12711226 + */ +#define XXUNIQUE_NAME(name, counter) name ## counter +#define XUNIQUE_NAME(name, counter) XXUNIQUE_NAME (name, counter) +#define NBDKIT_UNIQUE_NAME(name) XUNIQUE_NAME (name, __COUNTER__) #endif /* NBDKIT_UNIQUE_NAME_H */ diff --git a/common/utils/cleanup.h b/common/utils/cleanup.h index f38587e7..6ff7a24a 100644 --- a/common/utils/cleanup.h +++ b/common/utils/cleanup.h @@ -53,9 +53,11 @@ extern void cleanup_mutex_unlock (pthread_mutex_t **ptr); #define CLEANUP_MUTEX_UNLOCK __attribute__((cleanup (cleanup_mutex_unlock))) #define ACQUIRE_LOCK_FOR_CURRENT_SCOPE(mutex) \ - CLEANUP_MUTEX_UNLOCK pthread_mutex_t *NBDKIT_UNIQUE_NAME(_lock) = mutex; \ + ACQUIRE_LOCK_FOR_CURRENT_SCOPE_1((mutex), NBDKIT_UNIQUE_NAME(_lock)) +#define ACQUIRE_LOCK_FOR_CURRENT_SCOPE_1(mutex, lock) \ + CLEANUP_MUTEX_UNLOCK pthread_mutex_t *lock = mutex; \ do { \ - int _r = pthread_mutex_lock (NBDKIT_UNIQUE_NAME(_lock)); \ + int _r = pthread_mutex_lock (lock); \ assert (!_r); \ } while (0) @@ -63,17 +65,21 @@ extern void cleanup_rwlock_unlock (pthread_rwlock_t **ptr); #define CLEANUP_RWLOCK_UNLOCK __attribute__((cleanup (cleanup_rwlock_unlock))) #define ACQUIRE_WRLOCK_FOR_CURRENT_SCOPE(rwlock) \ - CLEANUP_RWLOCK_UNLOCK pthread_rwlock_t *NBDKIT_UNIQUE_NAME(_rwlock) = rwlock; \ - do { \ - int _r = pthread_rwlock_wrlock (NBDKIT_UNIQUE_NAME(_rwlock)); \ - assert (!_r); \ + ACQUIRE_WRLOCK_FOR_CURRENT_SCOPE_1((rwlock), NBDKIT_UNIQUE_NAME(_lock)) +#define ACQUIRE_WRLOCK_FOR_CURRENT_SCOPE_1(rwlock, lock) \ + CLEANUP_RWLOCK_UNLOCK pthread_rwlock_t *lock = rwlock; \ + do { \ + int _r = pthread_rwlock_wrlock (lock); \ + assert (!_r); \ } while (0) #define ACQUIRE_RDLOCK_FOR_CURRENT_SCOPE(rwlock) \ - CLEANUP_RWLOCK_UNLOCK pthread_rwlock_t *NBDKIT_UNIQUE_NAME(_rwlock) = rwlock; \ - do { \ - int _r = pthread_rwlock_rdlock (NBDKIT_UNIQUE_NAME(_rwlock)); \ - assert (!_r); \ + ACQUIRE_RDLOCK_FOR_CURRENT_SCOPE_1((rwlock), NBDKIT_UNIQUE_NAME(_lock)) +#define ACQUIRE_RDLOCK_FOR_CURRENT_SCOPE_1(rwlock, lock) \ + CLEANUP_RWLOCK_UNLOCK pthread_rwlock_t *lock = rwlock; \ + do { \ + int _r = pthread_rwlock_rdlock (lock); \ + assert (!_r); \ } while (0) /* cleanup-nbdkit.c */ -- 2.35.1

On 02/23/22 00:05, Eric Blake wrote: 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

Thanks: Eric Blake --- common/include/minmax.h | 21 +++++++++++++-------- common/include/test-minmax.c | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/common/include/minmax.h b/common/include/minmax.h index d3e667ea..8d1fbf19 100644 --- a/common/include/minmax.h +++ b/common/include/minmax.h @@ -44,16 +44,23 @@ * is required. */ +#include "unique-name.h" + +#undef MIN +#define MIN(x, y) \ + MIN_1((x), (y), NBDKIT_UNIQUE_NAME(_x), NBDKIT_UNIQUE_NAME(_y)) +#undef MAX +#define MAX(x, y) \ + MAX_1((x), (y), NBDKIT_UNIQUE_NAME(_x), NBDKIT_UNIQUE_NAME(_y)) + #ifdef HAVE_AUTO_TYPE -#undef MIN -#define MIN(x, y) ({ \ +#define MIN_1(x, y, _x, _y) ({ \ __auto_type _x = (x); \ __auto_type _y = (y); \ _x < _y ? _x : _y; \ }) -#undef MAX -#define MAX(x, y) ({ \ +#define MAX_1(x, y, _x, _y) ({ \ __auto_type _x = (x); \ __auto_type _y = (y); \ _x > _y ? _x : _y; \ @@ -61,14 +68,12 @@ #else -#undef MIN -#define MIN(x, y) ({ \ +#define MIN_1(x, y, _x, _y) ({ \ typeof (x) _x = (x); \ typeof (y) _y = (y); \ _x < _y ? _x : _y; \ }) -#undef MAX -#define MAX(x, y) ({ \ +#define MAX_1(x, y, _x, _y) ({ \ typeof (x) _x = (x); \ typeof (y) _y = (y); \ _x > _y ? _x : _y; \ diff --git a/common/include/test-minmax.c b/common/include/test-minmax.c index 7584ab27..32bc8a11 100644 --- a/common/include/test-minmax.c +++ b/common/include/test-minmax.c @@ -154,5 +154,19 @@ main (void) SIGNED_TEST (f, -FLT_MAX, FLT_MAX); SIGNED_TEST (d, -DBL_MAX, DBL_MAX); + /* Test that MIN and MAX can be nested. This is really a compile + * test, but we do check the answer. + */ + assert (MIN (MIN (1, 2), 3) == 1); + assert (MAX (MIN (1, 2), 3) == 3); + assert (MIN (MAX (1, 2), 3) == 2); + assert (MAX (MAX (1, 4), 3) == 4); + assert (MIN (3, MIN (1, 2)) == 1); + assert (MAX (3, MIN (1, 2)) == 3); + assert (MIN (3, MAX (1, 2)) == 2); + assert (MAX (3, MAX (1, 4)) == 4); + assert (MIN (MIN (1, MIN (2, 3)), 4) == 1); + assert (MAX (MAX (1, MAX (2, 3)), 4) == 4); + exit (EXIT_SUCCESS); } -- 2.35.1