core: Simplify thread abstractions and add debug checks

The POSIX thread mutex initialization function can potentially fail, but
in practice this is unlikely to occur. There is also inconsistent use of
the result of the mutex initialization within the library. The result is
only checked for mutexes in the libusb_device and libusb_device_handle
structures but is ignored in all other cases.

Simplify the mutex initialization function by changing the abstraction's
wrapper to a void function, much like all the other functions that
already exist. To that end, introduce macros for the abstractions that
will check the return value on debug builds.

Also remove the dependence on the core library needing errno.h to
translate errors from usbi_cond_timedwait(). The abstraction will
convert the implementation-specific error codes to LIBUSB_ERROR values.

Signed-off-by: Chris Dickens <christopher.a.dickens@gmail.com>

diff --git a/libusb/core.c b/libusb/core.c
index bac340ba9fe3f8cf811ab621744caf27723cab19..e103724becb28c612046d4e27ec2cbc104bb4d08 100644 (file)
--- a/libusb/core.c
+++ b/libusb/core.c
@@ -700,16 +700,11 @@ struct libusb_device *usbi_alloc_device(struct libusb_context *ctx,
 {
        size_t priv_size = usbi_backend.device_priv_size;
        struct libusb_device *dev = calloc(1, PTR_ALIGN(sizeof(*dev)) + priv_size);
-       int r;
 
        if (!dev)
                return NULL;
 
-       r = usbi_mutex_init(&dev->lock);
-       if (r) {
-               free(dev);
-               return NULL;
-       }
+       usbi_mutex_init(&dev->lock);
 
        dev->ctx = ctx;
        dev->refcnt = 1;
@@ -1267,11 +1262,7 @@ int API_EXPORTED libusb_wrap_sys_device(libusb_context *ctx, intptr_t sys_dev,
        if (!_dev_handle)
                return LIBUSB_ERROR_NO_MEM;
 
-       r = usbi_mutex_init(&_dev_handle->lock);
-       if (r) {
-               free(_dev_handle);
-               return LIBUSB_ERROR_OTHER;
-       }
+       usbi_mutex_init(&_dev_handle->lock);
 
        r = usbi_backend.wrap_sys_device(ctx, _dev_handle, sys_dev);
        if (r < 0) {
@@ -1325,11 +1316,7 @@ int API_EXPORTED libusb_open(libusb_device *dev,
        if (!_dev_handle)
                return LIBUSB_ERROR_NO_MEM;
 
-       r = usbi_mutex_init(&_dev_handle->lock);
-       if (r) {
-               free(_dev_handle);
-               return LIBUSB_ERROR_OTHER;
-       }
+       usbi_mutex_init(&_dev_handle->lock);
 
        _dev_handle->dev = libusb_ref_device(dev);
 

diff --git a/libusb/io.c b/libusb/io.c
index ebad3831958f2ed96bc665f8bfbc6ba7086e0fcf..b6256209d7590b4b41741483edc92694b317b8e1 100644 (file)
--- a/libusb/io.c
+++ b/libusb/io.c
@@ -1793,7 +1793,7 @@ int API_EXPORTED libusb_try_lock_events(libusb_context *ctx)
        }
 
        r = usbi_mutex_trylock(&ctx->events_lock);
-       if (r)
+       if (!r)
                return 1;
 
        ctx->event_handler_active = 1;
@@ -2016,11 +2016,10 @@ int API_EXPORTED libusb_wait_for_event(libusb_context *ctx, struct timeval *tv)
 
        r = usbi_cond_timedwait(&ctx->event_waiters_cond,
                &ctx->event_waiters_lock, tv);
-
        if (r < 0)
-               return r;
-       else
-               return (r == ETIMEDOUT);
+               return r == LIBUSB_ERROR_TIMEOUT;
+
+       return 0;
 }
 
 static void handle_timeout(struct usbi_transfer *itransfer)

diff --git a/libusb/os/threads_posix.c b/libusb/os/threads_posix.c
index 071e20590dd0f7036ce9cb32a12fe4afd6a35161..d79e8a6c86bac97bae39800207048b2048e9012d 100644 (file)
--- a/libusb/os/threads_posix.c
+++ b/libusb/os/threads_posix.c
@@ -21,6 +21,7 @@
 
 #include "libusbi.h"
 
+#include <errno.h>
 #if defined(__ANDROID__)
 # include <unistd.h>
 #elif defined(__HAIKU__)
@@ -55,15 +56,22 @@ int usbi_cond_timedwait(pthread_cond_t *cond,
                timeout.tv_sec++;
        }
 
-       return pthread_cond_timedwait(cond, mutex, &timeout);
+       r = pthread_cond_timedwait(cond, mutex, &timeout);
+       if (r == 0)
+               return 0;
+       else if (r == ETIMEDOUT)
+               return LIBUSB_ERROR_TIMEOUT;
+       else
+               return LIBUSB_ERROR_OTHER;
 }
 
-int usbi_get_tid(void)
+unsigned int usbi_get_tid(void)
 {
-       static _Thread_local int tid;
+       static _Thread_local unsigned int tl_tid;
+       int tid;
 
-       if (tid)
-               return tid;
+       if (tl_tid)
+               return tl_tid;
 
 #if defined(__ANDROID__)
        tid = gettid();
@@ -101,5 +109,5 @@ int usbi_get_tid(void)
                tid = (int)(intptr_t)pthread_self();
        }
 
-       return tid;
+       return tl_tid = (unsigned int)tid;
 }

diff --git a/libusb/os/threads_posix.h b/libusb/os/threads_posix.h
index eadb9783389be783d36aa114ad58f4a9d867c393..e187c69ea47cec2a7162d056203496c1fecfeee2 100644 (file)
--- a/libusb/os/threads_posix.h
+++ b/libusb/os/threads_posix.h
@@ -23,63 +23,69 @@
 
 #include <pthread.h>
 
+#define PTHREAD_CHECK(expr)                    \
+       do {                                    \
+               int pthread_result = (expr);    \
+               assert(pthread_result == 0);    \
+       } while (0)
+
 #define USBI_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
 typedef pthread_mutex_t usbi_mutex_static_t;
 static inline void usbi_mutex_static_lock(usbi_mutex_static_t *mutex)
 {
-       (void)pthread_mutex_lock(mutex);
+       PTHREAD_CHECK(pthread_mutex_lock(mutex));
 }
 static inline void usbi_mutex_static_unlock(usbi_mutex_static_t *mutex)
 {
-       (void)pthread_mutex_unlock(mutex);
+       PTHREAD_CHECK(pthread_mutex_unlock(mutex));
 }
 
 typedef pthread_mutex_t usbi_mutex_t;
-static inline int usbi_mutex_init(usbi_mutex_t *mutex)
+static inline void usbi_mutex_init(usbi_mutex_t *mutex)
 {
-       return pthread_mutex_init(mutex, NULL);
+       PTHREAD_CHECK(pthread_mutex_init(mutex, NULL));
 }
 static inline void usbi_mutex_lock(usbi_mutex_t *mutex)
 {
-       (void)pthread_mutex_lock(mutex);
+       PTHREAD_CHECK(pthread_mutex_lock(mutex));
 }
 static inline void usbi_mutex_unlock(usbi_mutex_t *mutex)
 {
-       (void)pthread_mutex_unlock(mutex);
+       PTHREAD_CHECK(pthread_mutex_unlock(mutex));
 }
 static inline int usbi_mutex_trylock(usbi_mutex_t *mutex)
 {
-       return pthread_mutex_trylock(mutex);
+       return pthread_mutex_trylock(mutex) == 0;
 }
 static inline void usbi_mutex_destroy(usbi_mutex_t *mutex)
 {
-       (void)pthread_mutex_destroy(mutex);
+       PTHREAD_CHECK(pthread_mutex_destroy(mutex));
 }
 
 typedef pthread_cond_t usbi_cond_t;
 static inline void usbi_cond_init(pthread_cond_t *cond)
 {
-       (void)pthread_cond_init(cond, NULL);
+       PTHREAD_CHECK(pthread_cond_init(cond, NULL));
 }
 static inline void usbi_cond_wait(usbi_cond_t *cond, usbi_mutex_t *mutex)
 {
-       (void)pthread_cond_wait(cond, mutex);
+       PTHREAD_CHECK(pthread_cond_wait(cond, mutex));
 }
 int usbi_cond_timedwait(usbi_cond_t *cond,
        usbi_mutex_t *mutex, const struct timeval *tv);
 static inline void usbi_cond_broadcast(usbi_cond_t *cond)
 {
-       (void)pthread_cond_broadcast(cond);
+       PTHREAD_CHECK(pthread_cond_broadcast(cond));
 }
 static inline void usbi_cond_destroy(usbi_cond_t *cond)
 {
-       (void)pthread_cond_destroy(cond);
+       PTHREAD_CHECK(pthread_cond_destroy(cond));
 }
 
 typedef pthread_key_t usbi_tls_key_t;
 static inline void usbi_tls_key_create(usbi_tls_key_t *key)
 {
-       (void)pthread_key_create(key, NULL);
+       PTHREAD_CHECK(pthread_key_create(key, NULL));
 }
 static inline void *usbi_tls_key_get(usbi_tls_key_t key)
 {
@@ -87,13 +93,13 @@ static inline void *usbi_tls_key_get(usbi_tls_key_t key)
 }
 static inline void usbi_tls_key_set(usbi_tls_key_t key, void *ptr)
 {
-       (void)pthread_setspecific(key, ptr);
+       PTHREAD_CHECK(pthread_setspecific(key, ptr));
 }
 static inline void usbi_tls_key_delete(usbi_tls_key_t key)
 {
-       (void)pthread_key_delete(key);
+       PTHREAD_CHECK(pthread_key_delete(key));
 }
 
-int usbi_get_tid(void);
+unsigned int usbi_get_tid(void);
 
 #endif /* LIBUSB_THREADS_POSIX_H */

diff --git a/libusb/os/threads_windows.c b/libusb/os/threads_windows.c
index cf726942efc49229a5c527c60fd8e3c311863a22..4a57f42ebaff80bbb8774425049c3c1fcac53372 100644 (file)
--- a/libusb/os/threads_windows.c
+++ b/libusb/os/threads_windows.c
@@ -34,7 +34,7 @@ int usbi_cond_timedwait(usbi_cond_t *cond,
        if (SleepConditionVariableCS(cond, mutex, millis))
                return 0;
        else if (GetLastError() == ERROR_TIMEOUT)
-               return ETIMEDOUT;
+               return LIBUSB_ERROR_TIMEOUT;
        else
-               return EINVAL;
+               return LIBUSB_ERROR_OTHER;
 }

diff --git a/libusb/os/threads_windows.h b/libusb/os/threads_windows.h
index 618c2f3da363fbc496850526a9f051fc03c50055..ed57f7ff74dff1ff4c751a86ada7fb5e8e05e957 100644 (file)
--- a/libusb/os/threads_windows.h
+++ b/libusb/os/threads_windows.h
@@ -21,7 +21,11 @@
 #ifndef LIBUSB_THREADS_WINDOWS_H
 #define LIBUSB_THREADS_WINDOWS_H
 
-#include <errno.h>
+#define WINAPI_CHECK(expr)                     \
+       do {                                    \
+               BOOL winapi_result = (expr);    \
+               assert(winapi_result != 0);     \
+       } while (0)
 
 #define USBI_MUTEX_INITIALIZER 0L
 typedef LONG usbi_mutex_static_t;
@@ -36,10 +40,9 @@ static inline void usbi_mutex_static_unlock(usbi_mutex_static_t *mutex)
 }
 
 typedef CRITICAL_SECTION usbi_mutex_t;
-static inline int usbi_mutex_init(usbi_mutex_t *mutex)
+static inline void usbi_mutex_init(usbi_mutex_t *mutex)
 {
        InitializeCriticalSection(mutex);
-       return 0;
 }
 static inline void usbi_mutex_lock(usbi_mutex_t *mutex)
 {
@@ -51,14 +54,13 @@ static inline void usbi_mutex_unlock(usbi_mutex_t *mutex)
 }
 static inline int usbi_mutex_trylock(usbi_mutex_t *mutex)
 {
-       return !TryEnterCriticalSection(mutex);
+       return TryEnterCriticalSection(mutex) != 0;
 }
 static inline void usbi_mutex_destroy(usbi_mutex_t *mutex)
 {
        DeleteCriticalSection(mutex);
 }
 
-// We *were* getting timespec from pthread.h:
 #if !defined(HAVE_STRUCT_TIMESPEC) && !defined(_TIMESPEC_DEFINED)
 #define HAVE_STRUCT_TIMESPEC 1
 #define _TIMESPEC_DEFINED 1
@@ -68,11 +70,6 @@ struct timespec {
 };
 #endif /* HAVE_STRUCT_TIMESPEC || _TIMESPEC_DEFINED */
 
-// We *were* getting ETIMEDOUT from pthread.h:
-#ifndef ETIMEDOUT
-#define ETIMEDOUT      10060   /* This is the value in winsock.h. */
-#endif
-
 typedef CONDITION_VARIABLE usbi_cond_t;
 static inline void usbi_cond_init(usbi_cond_t *cond)
 {
@@ -80,7 +77,7 @@ static inline void usbi_cond_init(usbi_cond_t *cond)
 }
 static inline void usbi_cond_wait(usbi_cond_t *cond, usbi_mutex_t *mutex)
 {
-       (void)SleepConditionVariableCS(cond, mutex, INFINITE);
+       WINAPI_CHECK(SleepConditionVariableCS(cond, mutex, INFINITE));
 }
 int usbi_cond_timedwait(usbi_cond_t *cond,
        usbi_mutex_t *mutex, const struct timeval *tv);
@@ -97,6 +94,7 @@ typedef DWORD usbi_tls_key_t;
 static inline void usbi_tls_key_create(usbi_tls_key_t *key)
 {
        *key = TlsAlloc();
+       assert(*key != TLS_OUT_OF_INDEXES);
 }
 static inline void *usbi_tls_key_get(usbi_tls_key_t key)
 {
@@ -104,16 +102,16 @@ static inline void *usbi_tls_key_get(usbi_tls_key_t key)
 }
 static inline void usbi_tls_key_set(usbi_tls_key_t key, void *ptr)
 {
-       (void)TlsSetValue(key, ptr);
+       WINAPI_CHECK(TlsSetValue(key, ptr));
 }
 static inline void usbi_tls_key_delete(usbi_tls_key_t key)
 {
-       (void)TlsFree(key);
+       WINAPI_CHECK(TlsFree(key));
 }
 
-static inline int usbi_get_tid(void)
+static inline unsigned int usbi_get_tid(void)
 {
-       return (int)GetCurrentThreadId();
+       return (unsigned int)GetCurrentThreadId();
 }
 
 #endif /* LIBUSB_THREADS_WINDOWS_H */

diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index 41b9eb49199c95780d0d7084e5a5312855aa7dd5..65fa0d758e6223d74accaae71b6473fa6fa3893e 100644 (file)
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11554
+#define LIBUSB_NANO 11555