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>

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

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)

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

libusb/os/threads_posix.h

@@ -23,77 +23,83 @@
 
 #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)
 {
 	return pthread_getspecific(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 */

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

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,19 +70,14 @@ 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)
 {
 	InitializeConditionVariable(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,23 +94,24 @@ 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)
 {
 	return TlsGetValue(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 */

libusb/version_nano.h

@@ -1 +1 @@
-#define LIBUSB_NANO 11554
+#define LIBUSB_NANO 11555