diff mbox series

[6/8] include/qemu/lockable: Use _Generic instead of QEMU_GENERIC

Message ID 20210611233347.653129-7-richard.henderson@linaro.org (mailing list archive)
State New, archived
Headers show
Series configure: Change to -std=gnu11 | expand

Commit Message

Richard Henderson June 11, 2021, 11:33 p.m. UTC 
From: Richard Henderson <richard.henderson@liaro.org>

This is both more and less complicated than our expansion
using __builtin_choose_expr and __builtin_types_compatible_p.

The expansion through QEMU_MAKE_LOCKABLE_ doesn't work because
we're not emumerating all of the types within the same _Generic,
which results in errors about unhandled cases.  We must also
handle void* explicitly, so that the NULL constant can be used.

Signed-off-by: Richard Henderson <richard.henderson@liaro.org>
---
 include/qemu/lockable.h | 85 +++++++++++++++++++++--------------------
 1 file changed, 43 insertions(+), 42 deletions(-)

Comments

Paolo Bonzini June 14, 2021, 11:14 a.m. UTC | #1 
On 12/06/21 01:33, Richard Henderson wrote:
> -                 unknown_lock_type))
> +/* Auxiliary macros to simplify QEMU_MAKE_LOCABLE.  */
> +#define QEMU_LOCK_FUNC(x) ((QemuLockUnlockFunc *)      \
> +    _Generic((x), QemuMutex *: qemu_mutex_lock,        \
> +                  QemuRecMutex *: qemu_rec_mutex_lock, \
> +                  CoMutex *: qemu_co_mutex_lock,       \
> +                  QemuSpin *: qemu_spin_lock))
> +
> +#define QEMU_UNLOCK_FUNC(x) ((QemuLockUnlockFunc *)      \
> +    _Generic((x), QemuMutex *: qemu_mutex_unlock,        \
> +                  QemuRecMutex *: qemu_rec_mutex_unlock, \
> +                  CoMutex *: qemu_co_mutex_unlock,       \
> +                  QemuSpin *: qemu_spin_unlock))
>   

These are not needed anymore, are they?

Otherwise I agree that it's both more and less complicated.  The 
duplication between QEMU_MAKE_LOCKABLE_NONNULL and QEMU_MAKE_LOCKABLE is 
tolerable.

Thanks,

Paolo
Richard Henderson June 14, 2021, 2:47 p.m. UTC | #2 
On 6/14/21 4:14 AM, Paolo Bonzini wrote:
> On 12/06/21 01:33, Richard Henderson wrote:
>> -                 unknown_lock_type))
>> +/* Auxiliary macros to simplify QEMU_MAKE_LOCABLE.  */
>> +#define QEMU_LOCK_FUNC(x) ((QemuLockUnlockFunc *)      \
>> +    _Generic((x), QemuMutex *: qemu_mutex_lock,        \
>> +                  QemuRecMutex *: qemu_rec_mutex_lock, \
>> +                  CoMutex *: qemu_co_mutex_lock,       \
>> +                  QemuSpin *: qemu_spin_lock))
>> +
>> +#define QEMU_UNLOCK_FUNC(x) ((QemuLockUnlockFunc *)      \
>> +    _Generic((x), QemuMutex *: qemu_mutex_unlock,        \
>> +                  QemuRecMutex *: qemu_rec_mutex_unlock, \
>> +                  CoMutex *: qemu_co_mutex_unlock,       \
>> +                  QemuSpin *: qemu_spin_unlock))
> 
> These are not needed anymore, are they?
> 
> Otherwise I agree that it's both more and less complicated.  The duplication between 
> QEMU_MAKE_LOCKABLE_NONNULL and QEMU_MAKE_LOCKABLE is tolerable.

Yep, I didn't notice that they were only used by MAKE_LOCKABLE.

r~
diff mbox series

Patch

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index b620023141..9118d54200 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -24,19 +24,6 @@  struct QemuLockable {
     QemuLockUnlockFunc *unlock;
 };
 
-/* This function gives an error if an invalid, non-NULL pointer type is passed
- * to QEMU_MAKE_LOCKABLE.  For optimized builds, we can rely on dead-code elimination
- * from the compiler, and give the errors already at link time.
- */
-#if defined(__OPTIMIZE__) && !defined(__SANITIZE_ADDRESS__)
-void unknown_lock_type(void *);
-#else
-static inline void unknown_lock_type(void *unused)
-{
-    abort();
-}
-#endif
-
 static inline __attribute__((__always_inline__)) QemuLockable *
 qemu_make_lockable(void *x, QemuLockable *lockable)
 {
@@ -46,57 +33,71 @@  qemu_make_lockable(void *x, QemuLockable *lockable)
     return x ? lockable : NULL;
 }
 
-/* Auxiliary macros to simplify QEMU_MAKE_LOCABLE.  */
-#define QEMU_LOCK_FUNC(x) ((QemuLockUnlockFunc *)    \
-    QEMU_GENERIC(x,                                  \
-                 (QemuMutex *, qemu_mutex_lock),     \
-                 (QemuRecMutex *, qemu_rec_mutex_lock), \
-                 (CoMutex *, qemu_co_mutex_lock),    \
-                 (QemuSpin *, qemu_spin_lock),       \
-                 unknown_lock_type))
+static inline __attribute__((__always_inline__)) QemuLockable *
+qemu_null_lockable(void *x)
+{
+    if (x != NULL) {
+        qemu_build_not_reached();
+    }
+    return NULL;
+}
 
-#define QEMU_UNLOCK_FUNC(x) ((QemuLockUnlockFunc *)  \
-    QEMU_GENERIC(x,                                  \
-                 (QemuMutex *, qemu_mutex_unlock),   \
-                 (QemuRecMutex *, qemu_rec_mutex_unlock), \
-                 (CoMutex *, qemu_co_mutex_unlock),  \
-                 (QemuSpin *, qemu_spin_unlock),     \
-                 unknown_lock_type))
+/* Auxiliary macros to simplify QEMU_MAKE_LOCABLE.  */
+#define QEMU_LOCK_FUNC(x) ((QemuLockUnlockFunc *)      \
+    _Generic((x), QemuMutex *: qemu_mutex_lock,        \
+                  QemuRecMutex *: qemu_rec_mutex_lock, \
+                  CoMutex *: qemu_co_mutex_lock,       \
+                  QemuSpin *: qemu_spin_lock))
+
+#define QEMU_UNLOCK_FUNC(x) ((QemuLockUnlockFunc *)      \
+    _Generic((x), QemuMutex *: qemu_mutex_unlock,        \
+                  QemuRecMutex *: qemu_rec_mutex_unlock, \
+                  CoMutex *: qemu_co_mutex_unlock,       \
+                  QemuSpin *: qemu_spin_unlock))
 
 /* In C, compound literals have the lifetime of an automatic variable.
  * In C++ it would be different, but then C++ wouldn't need QemuLockable
  * either...
  */
-#define QEMU_MAKE_LOCKABLE_(x) (&(QemuLockable) {     \
-        .object = (x),                               \
-        .lock = QEMU_LOCK_FUNC(x),                   \
-        .unlock = QEMU_UNLOCK_FUNC(x),               \
+#define QML_OBJ_(x, name) (&(QemuLockable) {                            \
+        .object = (x),                                                  \
+        .lock = (QemuLockUnlockFunc *) qemu_ ## name ## _lock,          \
+        .unlock = (QemuLockUnlockFunc *) qemu_ ## name ## _unlock       \
     })
 
 /* QEMU_MAKE_LOCKABLE - Make a polymorphic QemuLockable
  *
- * @x: a lock object (currently one of QemuMutex, QemuRecMutex, CoMutex, QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
+ *     CoMutex, QemuSpin).
  *
  * Returns a QemuLockable object that can be passed around
  * to a function that can operate with locks of any kind, or
  * NULL if @x is %NULL.
+ *
+ * Note the special case for void *, so that we may pass "NULL".
  */
-#define QEMU_MAKE_LOCKABLE(x)                        \
-    QEMU_GENERIC(x,                                  \
-                 (QemuLockable *, (x)),              \
-                 qemu_make_lockable((x), QEMU_MAKE_LOCKABLE_(x)))
+#define QEMU_MAKE_LOCKABLE(x)                                           \
+    _Generic((x), QemuLockable *: (x),                                  \
+             void *: qemu_null_lockable(x),                             \
+             QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)),    \
+             QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x, rec_mutex)), \
+             CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),   \
+             QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
 
 /* QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
  *
- * @x: a lock object (currently one of QemuMutex, QemuRecMutex, CoMutex, QemuSpin).
+ * @x: a lock object (currently one of QemuMutex, QemuRecMutex,
+ *     CoMutex, QemuSpin).
  *
  * Returns a QemuLockable object that can be passed around
  * to a function that can operate with locks of any kind.
  */
-#define QEMU_MAKE_LOCKABLE_NONNULL(x)                \
-    QEMU_GENERIC(x,                                  \
-                 (QemuLockable *, (x)),              \
-                 QEMU_MAKE_LOCKABLE_(x))
+#define QEMU_MAKE_LOCKABLE_NONNULL(x)                           \
+    _Generic((x), QemuLockable *: (x),                          \
+                  QemuMutex *: QML_OBJ_(x, mutex),              \
+                  QemuRecMutex *: QML_OBJ_(x, rec_mutex),       \
+                  CoMutex *: QML_OBJ_(x, co_mutex),             \
+                  QemuSpin *: QML_OBJ_(x, spin))
 
 static inline void qemu_lockable_lock(QemuLockable *x)
 {