[Qemu-devel] [RFC PATCH] osdep: Make MIN/MAX evaluate arguments only once

[Eric Blake]

Use the __auto_type keyword to make sure our min/max macros only
evaluate their arguments once.

Signed-off-by: Eric Blake <address@hidden>
---

RFC because __auto_type didn't exist until gcc 4.9, and I don't know
which clang version introduced it (other than that it went in
during 2015: https://reviews.llvm.org/D12686).  Our minimum gcc
version is 4.8, which has typeof; I'm not sure if our minimum clang
version supports typeof.

I'm considering adding a snippet to compiler.h that looks like:

 #if .... // new enough gcc/clang
 #define QEMU_TYPEOF(a) __auto_type
 #else
 #define QEMU_TYPEOF(a) typeof(a)
 #endif

at which point we could blindly use QEMU_TYPEOF(a)=(a) anywhere we
need automatic typing, for the benefit of smaller macro expansion
[and proper handling of VLA types, although I don't think we use
those to care about that aspect of __auto_type] in newer compilers,
while still getting automatic type deduction in older compilers for
macros that want single evaluation, and where we've localized the
version checks to one spot instead of everywhere.  But for that to
work, again, I need to know whether typeof is supported in our
minimum clang version, and how to properly spell the version check
for clang on when to prefer __auto_type over typeof (at least I
know how to spell it for gcc).

While at it, the comments to MIN_NON_ZERO() state that callers should
only compare unsigned types; I suspect we don't actually obey that
rule, but I also think the comment is over-strict - the macro works
as long as both arguments are non-negative, and when called with a
mix of signed and unsigned types, as long as the type promotion
preserves the fact that the value is still non-negative.  But it
might be interesting to add compile-time checking (or maybe runtime
asserts) that the macro is indeed only used on non-negative values.

 include/qemu/osdep.h | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 3bf48bcdec0..b941572b808 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -233,17 +233,31 @@ extern int daemon(int, int);
 #endif

 #ifndef MIN
-#define MIN(a, b) (((a) < (b)) ? (a) : (b))
+#define MIN(a, b)              \
+    ({                         \
+        __auto_type _a = (a);  \
+        __auto_type _b = (b);  \
+        _a < _b ? _a : _b;     \
+    })
 #endif
 #ifndef MAX
-#define MAX(a, b) (((a) > (b)) ? (a) : (b))
+#define MAX(a, b)              \
+    ({                         \
+        __auto_type _a = (a);  \
+        __auto_type _b = (b);  \
+        _a > _b ? _a : _b;     \
+    })
 #endif

 /* Minimum function that returns zero only iff both values are zero.
  * Intended for use with unsigned values only. */
 #ifndef MIN_NON_ZERO
-#define MIN_NON_ZERO(a, b) ((a) == 0 ? (b) : \
-                                ((b) == 0 ? (a) : (MIN(a, b))))
+#define MIN_NON_ZERO(a, b)                              \
+    ({                                                  \
+        __auto_type _a = (a);                           \
+        __auto_type _b = (b);                           \
+        _a == 0 ? _b : (_b == 0 || _b > _a) ? _a : _b;  \
+    })
 #endif

 /* Round number down to multiple */
-- 
2.20.1