Richard W.M. Jones
2022-Feb-22 21:35 UTC
[Libguestfs] [PATCH nbdkit 1/2] common/include: Change unique-name macros to use __COUNTER__
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
Eric Blake
2022-Feb-22 23:05 UTC
[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. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org