Richard W.M. Jones
2022-Feb-22 21:35 UTC
[Libguestfs] [PATCH nbdkit 0/2] common/include: Fix MIN and MAX macros so they can be nested
Not tested very much yet. I need to test them across a lot more platforms and compilers. However for a first cut they look good. Rich.
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
Richard W.M. Jones
2022-Feb-22 21:35 UTC
[Libguestfs] [PATCH nbdkit 2/2] common/include: Fix MIN and MAX macros so they can be nested
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
Richard W.M. Jones
2022-Feb-22 22:02 UTC
[Libguestfs] [PATCH nbdkit 0/2] common/include: Fix MIN and MAX macros so they can be nested
On Tue, Feb 22, 2022 at 09:35:02PM +0000, Richard W.M. Jones wrote:> Not tested very much yet. I need to test them across a lot more > platforms and compilers. However for a first cut they look good.I tested the patches now on: - FreeBSD - OpenBSD - clang on Linux - mingw-w64 (Fedora Windows cross compiler) and they work on all of those. Rich. -- 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