Re: [PATCH 2/2] lockable: do not rely on optimization for null lockables

[Eric Blake <eblake@redhat.com>]

On 6/3/20 5:49 PM, Joe Slater wrote:
> If we use QLNULL for null lockables, we can always
> use referencing unknown_lock_type as a link time
> error indicator.
> 
> Signed-off-by: Joe Slater <joe.slater@windriver.com>
> ---
>   include/qemu/lockable.h | 19 ++++++-------------
>   1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
> index 7f7aebb..7fc5750 100644
> --- a/include/qemu/lockable.h
> +++ b/include/qemu/lockable.h
> @@ -24,21 +24,14 @@ struct QemuLockable {
>       QemuLockUnlockFunc *unlock;
>   };
>   
> -#define QLNULL ((QemuLockable *)0)
> -
> -
> -/* 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 unknown_lock_type() is referenced, it means we have tried to passed something
> + *  not recognized as lockable to the macros below.  Use QLNULL to intentionally pass
> + *  a null lockable.
>    */
> -#if defined(__OPTIMIZE__) && !defined(__SANITIZE_ADDRESS__)
> +#define QLNULL ((QemuLockable *)0)

Looks like a bit of churn (especially if you take my comment on 1/2 to 
spell it NULL instead of 0).  Should these be combined into a single patch?

>   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)


Reading further in the file:

/* Auxiliary macros to simplify QEMU_MAKE_LOCABLE.  */

We should fix that typo while improving things.

/* QEMU_MAKE_LOCKABLE - Make a polymorphic QemuLockable
  *
  * @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.

Should this comment be tweaked to call out %QLNULL?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org