Libguestfs - Apr 2019 - [RFC: nbdkit PATCH] cleanup: Assert mutex sanity

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.

Signed-off-by: Eric Blake <eblake@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?

 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);
 }
-- 
2.20.1

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