[Libguestfs] [RFC: nbdkit PATCH] cleanup: Assert mutex sanity

[Richard W.M. Jones]

On Tue, Apr 23, 2019 at 03:07:14PM -0500, Eric Blake wrote:
> Although we haven't always checked that pthread_mutex_[un]lock
> succeeded, it never hurts to avoid blatant programming bugs such as
> when EINVAL can detect use of an uninitialized mutex.

I see no reason not to do this, so ACK.  I'll push this one as well.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
> 
> Applies on top of my series to move cleanup.c to common/
> 
> Should I also go through and add checking to other bare
> pthread_mutex_[un]lock() calls?

Yes please!

Thanks,

Rich.

>  common/utils/cleanup.h | 5 ++++-
>  common/utils/cleanup.c | 3 ++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/common/utils/cleanup.h b/common/utils/cleanup.h
> index e6e6140..0ab9e65 100644
> --- a/common/utils/cleanup.h
> +++ b/common/utils/cleanup.h
> @@ -43,6 +43,9 @@ extern void cleanup_unlock (pthread_mutex_t **ptr);
>  #define CLEANUP_UNLOCK __attribute__((cleanup (cleanup_unlock)))
>  #define ACQUIRE_LOCK_FOR_CURRENT_SCOPE(mutex) \
>    CLEANUP_UNLOCK pthread_mutex_t *_lock = mutex; \
> -  pthread_mutex_lock (_lock)
> +  do { \
> +    int _r = pthread_mutex_lock (_lock); \
> +    assert (!_r); \
> +  } while (0)
> 
>  #endif /* NBDKIT_CLEANUP_H */
> diff --git a/common/utils/cleanup.c b/common/utils/cleanup.c
> index 196d910..995f46c 100644
> --- a/common/utils/cleanup.c
> +++ b/common/utils/cleanup.c
> @@ -53,5 +53,6 @@ cleanup_extents_free (void *ptr)
>  void
>  cleanup_unlock (pthread_mutex_t **ptr)
>  {
> -  pthread_mutex_unlock (*ptr);
> +  int r = pthread_mutex_unlock (*ptr);
> +  assert (!r);
>  }



-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org