Windows: Fix style issues in threading code

Also moves private structure definition outside of header file.

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

3 files changed, 189 insertions, 130 deletions

diff --git a/libusb/os/threads_windows.c b/libusb/os/threads_windows.c
index e3fd910..933807e 100644
--- a/libusb/os/threads_windows.c
+++ b/libusb/os/threads_windows.c
@@ -22,134 +22,183 @@
 
 #include <objbase.h>
 #include <errno.h>
-#include <stdarg.h>
 
 #include "libusbi.h"
 
 extern const uint64_t epoch_time;
 
-int usbi_mutex_init(usbi_mutex_t *mutex,
-					const usbi_mutexattr_t *attr) {
-	UNUSED(attr);
-	if(! mutex) return ((errno=EINVAL));
-	*mutex = CreateMutex(NULL, FALSE, NULL);
-	if(!*mutex) return ((errno=ENOMEM));
+struct usbi_cond_perthread {
+	struct list_head list;
+	DWORD tid;
+	HANDLE event;
+};
+
+int usbi_mutex_static_lock(usbi_mutex_static_t *mutex)
+{
+	if (!mutex)
+		return EINVAL;
+	while (InterlockedExchange(mutex, 1) == 1)
+		SleepEx(0, TRUE);
 	return 0;
 }
-int usbi_mutex_destroy(usbi_mutex_t *mutex) {
-	// It is not clear if CloseHandle failure is due to failure to unlock.
-	//   If so, this should be errno=EBUSY.
-	if(!mutex || !CloseHandle(*mutex)) return ((errno=EINVAL));
-	*mutex = NULL;
+int usbi_mutex_static_unlock(usbi_mutex_static_t *mutex)
+{
+	if (!mutex)
+		return EINVAL;
+	InterlockedExchange(mutex, 0);
 	return 0;
 }
-int usbi_mutex_trylock(usbi_mutex_t *mutex) {
-	DWORD result;
-	if(!mutex) return ((errno=EINVAL));
-	result = WaitForSingleObject(*mutex, 0);
-	if(result == WAIT_OBJECT_0 || result == WAIT_ABANDONED)
-		return 0; // acquired (ToDo: check that abandoned is ok)
-	if(result == WAIT_TIMEOUT)
-		return ((errno=EBUSY));
-	return ((errno=EINVAL)); // don't know how this would happen
-							 //   so don't know proper errno
+
+int usbi_mutex_init(usbi_mutex_t *mutex, const usbi_mutexattr_t *attr)
+{
+	UNUSED(attr);
+	if (!mutex)
+		return EINVAL;
+	*mutex = CreateMutex(NULL, FALSE, NULL);
+	if (!*mutex)
+		return ENOMEM;
+	return 0;
 }
-int usbi_mutex_lock(usbi_mutex_t *mutex) {
+
+int usbi_mutex_lock(usbi_mutex_t *mutex)
+{
 	DWORD result;
-	if(!mutex) return ((errno=EINVAL));
+
+	if (!mutex)
+		return EINVAL;
 	result = WaitForSingleObject(*mutex, INFINITE);
-	if(result == WAIT_OBJECT_0 || result == WAIT_ABANDONED)
+	if (result == WAIT_OBJECT_0 || result == WAIT_ABANDONED)
 		return 0; // acquired (ToDo: check that abandoned is ok)
-	return ((errno=EINVAL)); // don't know how this would happen
-							 //   so don't know proper errno
+	else
+		return EINVAL; // don't know how this would happen
+			       //   so don't know proper errno
 }
-int usbi_mutex_unlock(usbi_mutex_t *mutex) {
-	if(!mutex)                return ((errno=EINVAL));
-	if(!ReleaseMutex(*mutex)) return ((errno=EPERM ));
-	return 0;
+
+int usbi_mutex_unlock(usbi_mutex_t *mutex)
+{
+	if (!mutex)
+		return EINVAL;
+	if (ReleaseMutex(*mutex))
+		return 0;
+	else
+		return EPERM;
 }
 
-int usbi_mutex_static_lock(usbi_mutex_static_t *mutex) {
-	if(!mutex)               return ((errno=EINVAL));
-	while (InterlockedExchange((LONG *)mutex, 1) == 1) {
-		SleepEx(0, TRUE);
-	}
-	return 0;
+int usbi_mutex_trylock(usbi_mutex_t *mutex)
+{
+	DWORD result;
+
+	if (!mutex)
+		return EINVAL;
+	result = WaitForSingleObject(*mutex, 0);
+	if (result == WAIT_OBJECT_0 || result == WAIT_ABANDONED)
+		return 0; // acquired (ToDo: check that abandoned is ok)
+	else if (result == WAIT_TIMEOUT)
+		return EBUSY;
+	else
+		return EINVAL; // don't know how this would happen
+			       //   so don't know proper error
 }
-int usbi_mutex_static_unlock(usbi_mutex_static_t *mutex) {
-	if(!mutex)               return ((errno=EINVAL));
-	*mutex = 0;
+
+int usbi_mutex_destroy(usbi_mutex_t *mutex)
+{
+	// It is not clear if CloseHandle failure is due to failure to unlock.
+	//   If so, this should be errno=EBUSY.
+	if (!mutex || !CloseHandle(*mutex))
+		return EINVAL;
+	*mutex = NULL;
 	return 0;
 }
 
-int usbi_cond_init(usbi_cond_t *cond,
-				   const usbi_condattr_t *attr) {
+int usbi_cond_init(usbi_cond_t *cond, const usbi_condattr_t *attr)
+{
 	UNUSED(attr);
-	if(!cond)           return ((errno=EINVAL));
-	list_init(&cond->waiters    );
+	if (!cond)
+		return EINVAL;
+	list_init(&cond->waiters);
 	list_init(&cond->not_waiting);
 	return 0;
 }
-int usbi_cond_destroy(usbi_cond_t *cond) {
+
+int usbi_cond_destroy(usbi_cond_t *cond)
+{
 	// This assumes no one is using this anymore.  The check MAY NOT BE safe.
-	struct usbi_cond_perthread *pos, *next_pos = NULL;
-	if(!cond) return ((errno=EINVAL));
-	if(!list_empty(&cond->waiters)) return ((errno=EBUSY )); // (!see above!)
+	struct usbi_cond_perthread *pos, *next_pos;
+
+	if(!cond)
+		return EINVAL;
+	if (!list_empty(&cond->waiters))
+		return EBUSY; // (!see above!)
 	list_for_each_entry_safe(pos, next_pos, &cond->not_waiting, list, struct usbi_cond_perthread) {
 		CloseHandle(pos->event);
 		list_del(&pos->list);
 		free(pos);
 	}
-
 	return 0;
 }
 
-int usbi_cond_broadcast(usbi_cond_t *cond) {
+int usbi_cond_broadcast(usbi_cond_t *cond)
+{
 	// Assumes mutex is locked; this is not in keeping with POSIX spec, but
 	//   libusb does this anyway, so we simplify by not adding more sync
 	//   primitives to the CV definition!
 	int fail = 0;
 	struct usbi_cond_perthread *pos;
-	if(!cond)                      return ((errno=EINVAL));
+
+	if (!cond)
+		return EINVAL;
 	list_for_each_entry(pos, &cond->waiters, list, struct usbi_cond_perthread) {
-		if(!SetEvent(pos->event))
+		if (!SetEvent(pos->event))
 			fail = 1;
 	}
 	// The wait function will remove its respective item from the list.
-	return fail ? ((errno=EINVAL)) : 0;
+	return fail ? EINVAL : 0;
 }
-int usbi_cond_signal(usbi_cond_t *cond) {
+
+int usbi_cond_signal(usbi_cond_t *cond)
+{
 	// Assumes mutex is locked; this is not in keeping with POSIX spec, but
 	//   libusb does this anyway, so we simplify by not adding more sync
 	//   primitives to the CV definition!
 	struct usbi_cond_perthread *pos;
-	if(!cond)                      return ((errno=EINVAL));
-	if(list_empty(&cond->waiters)) return 0; // no one to wakeup.
-	pos = list_entry(&cond->waiters.next, struct usbi_cond_perthread, list);
+
+	if (!cond)
+		return EINVAL;
+	if (list_empty(&cond->waiters))
+		return 0; // no one to wakeup.
+	pos = list_first_entry(&cond->waiters, struct usbi_cond_perthread, list);
 	// The wait function will remove its respective item from the list.
-	return SetEvent(pos->event) ? 0 : ((errno=EINVAL));
+	if (SetEvent(pos->event))
+		return 0;
+	else
+		return EINVAL;
 }
+
 __inline static int usbi_cond_intwait(usbi_cond_t *cond,
-									  usbi_mutex_t *mutex,
-									  DWORD timeout_ms) {
+	usbi_mutex_t *mutex, DWORD timeout_ms)
+{
 	struct usbi_cond_perthread *pos;
-	int found = 0, r;
-	DWORD r2,tid = GetCurrentThreadId();
-	if(!cond || !mutex) return ((errno=EINVAL));
+	int r, found = 0;
+	DWORD r2, tid = GetCurrentThreadId();
+
+	if (!cond || !mutex)
+		return EINVAL;
 	list_for_each_entry(pos, &cond->not_waiting, list, struct usbi_cond_perthread) {
 		if(tid == pos->tid) {
 			found = 1;
 			break;
 		}
 	}
-	if(!found) {
-		pos      = (struct usbi_cond_perthread*) calloc(1, sizeof(struct usbi_cond_perthread));
-		if(!pos) return ((errno=ENOMEM)); // This errno is not POSIX-allowed.
+
+	if (!found) {
+		pos = calloc(1, sizeof(struct usbi_cond_perthread));
+		if (!pos)
+			return ENOMEM; // This errno is not POSIX-allowed.
 		pos->tid = tid;
 		pos->event = CreateEvent(NULL, FALSE, FALSE, NULL); // auto-reset.
-		if(!pos->event) {
+		if (!pos->event) {
 			free(pos);
-			return      ((errno=ENOMEM));
+			return ENOMEM;
 		}
 		list_add(&pos->list, &cond->not_waiting);
 	}
@@ -158,25 +207,33 @@ __inline static int usbi_cond_intwait(usbi_cond_t *cond,
 	list_add(&pos->list, &cond->waiters);
 
 	r  = usbi_mutex_unlock(mutex);
-	if(r) return r;
+	if (r)
+		return r;
+
 	r2 = WaitForSingleObject(pos->event, timeout_ms);
-	r  = usbi_mutex_lock(mutex);
-	if(r) return r;
+	r = usbi_mutex_lock(mutex);
+	if (r)
+		return r;
 
 	list_del(&pos->list);
 	list_add(&pos->list, &cond->not_waiting);
 
-	if(r2 == WAIT_TIMEOUT) return ((errno=ETIMEDOUT));
-
-	return 0;
+	if (r2 == WAIT_OBJECT_0)
+		return 0;
+	else if (r2 == WAIT_TIMEOUT)
+		return ETIMEDOUT;
+	else
+		return EINVAL;
 }
 // N.B.: usbi_cond_*wait() can also return ENOMEM, even though pthread_cond_*wait cannot!
-int usbi_cond_wait(usbi_cond_t *cond, usbi_mutex_t *mutex) {
+int usbi_cond_wait(usbi_cond_t *cond, usbi_mutex_t *mutex)
+{
 	return usbi_cond_intwait(cond, mutex, INFINITE);
 }
+
 int usbi_cond_timedwait(usbi_cond_t *cond,
-						usbi_mutex_t *mutex,
-						const struct timespec *abstime) {
+	usbi_mutex_t *mutex, const struct timespec *abstime)
+{
 	FILETIME filetime;
 	ULARGE_INTEGER rtime;
 	struct timeval targ_time, cur_time, delta_time;
@@ -192,45 +249,58 @@ int usbi_cond_timedwait(usbi_cond_t *cond,
 	rtime.HighPart  = filetime.dwHighDateTime;
 	rtime.QuadPart -= epoch_time;
 	cur_time_ns.tv_sec = (long)(rtime.QuadPart / 10000000);
-	cur_time_ns.tv_nsec = (long)((rtime.QuadPart % 10000000)*100);
+	cur_time_ns.tv_nsec = (long)((rtime.QuadPart % 10000000) * 100);
 	TIMESPEC_TO_TIMEVAL(&cur_time, &cur_time_ns);
 
 	TIMESPEC_TO_TIMEVAL(&targ_time, abstime);
 	timersub(&targ_time, &cur_time, &delta_time);
-	if(delta_time.tv_sec < 0) // abstime already passed?
+	if (delta_time.tv_sec < 0) {
+		// abstime already passed?
 		millis = 0;
-	else {
-		millis  = delta_time.tv_usec/1000;
-		millis += delta_time.tv_sec *1000;
-		if (delta_time.tv_usec % 1000) // round up to next millisecond
-			millis++;
+	} else {
+		millis  = delta_time.tv_usec / 1000;
+		millis += delta_time.tv_sec * 1000;
+		if (delta_time.tv_usec % 1000)
+			millis++; // round up to next millisecond
 	}
 
 	return usbi_cond_intwait(cond, mutex, millis);
 }
 
-int usbi_tls_key_create(usbi_tls_key_t *key, void (*destructor)(void *)) {
+int usbi_tls_key_create(usbi_tls_key_t *key, void (*destructor)(void *))
+{
 	UNUSED(destructor);
-	if(!key) return ((errno=EINVAL));
+	if (!key)
+		return EINVAL;
 	*key = TlsAlloc();
-	if (*key == TLS_OUT_OF_INDEXES) return ((errno=ENOMEM));
-	return 0;
+	if (*key == TLS_OUT_OF_INDEXES)
+		return ENOMEM;
+	else
+		return 0;
 }
 
-void *usbi_tls_key_get(usbi_tls_key_t key) {
+void *usbi_tls_key_get(usbi_tls_key_t key)
+{
 	return TlsGetValue(key);
 }
 
-int usbi_tls_key_set(usbi_tls_key_t key, const void *value) {
-	if (!TlsSetValue(key, (LPVOID)value)) return ((errno=EINVAL));
-	return 0;
+int usbi_tls_key_set(usbi_tls_key_t key, const void *value)
+{
+	if (TlsSetValue(key, (LPVOID)value))
+		return 0;
+	else
+		return EINVAL;
 }
 
-int usbi_tls_key_delete(usbi_tls_key_t key) {
-	if (!TlsFree(key)) return ((errno=EINVAL));
-	return 0;
+int usbi_tls_key_delete(usbi_tls_key_t key)
+{
+	if (TlsFree(key))
+		return 0;
+	else
+		return EINVAL;
 }
 
-int usbi_get_tid(void) {
-	return GetCurrentThreadId();
+int usbi_get_tid(void)
+{
+	return (int)GetCurrentThreadId();
 }
diff --git a/libusb/os/threads_windows.h b/libusb/os/threads_windows.h
index 6a35d9d..39dddc8 100644
--- a/libusb/os/threads_windows.h
+++ b/libusb/os/threads_windows.h
@@ -21,32 +21,25 @@
 #ifndef LIBUSB_THREADS_WINDOWS_H
 #define LIBUSB_THREADS_WINDOWS_H
 
-#define usbi_mutex_static_t     volatile LONG
-#define USBI_MUTEX_INITIALIZER  0
+#define usbi_mutex_static_t	volatile LONG
+#define USBI_MUTEX_INITIALIZER	0
 
-#define usbi_mutex_t            HANDLE
+#define usbi_mutex_t		HANDLE
 
-struct usbi_cond_perthread {
-	struct list_head list;
-	DWORD            tid;
-	HANDLE           event;
-};
-struct usbi_cond_t_ {
+typedef struct usbi_cond {
 	// Every time a thread touches the CV, it winds up in one of these lists.
-	//   It stays there until the CV is destroyed, even if the thread
-	//   terminates.
+	//   It stays there until the CV is destroyed, even if the thread terminates.
 	struct list_head waiters;
 	struct list_head not_waiting;
-};
-typedef struct usbi_cond_t_ usbi_cond_t;
+} usbi_cond_t;
 
 // We *were* getting timespec from pthread.h:
 #if (!defined(HAVE_STRUCT_TIMESPEC) && !defined(_TIMESPEC_DEFINED))
 #define HAVE_STRUCT_TIMESPEC 1
 #define _TIMESPEC_DEFINED 1
 struct timespec {
-		long tv_sec;
-		long tv_nsec;
+	long tv_sec;
+	long tv_nsec;
 };
 #endif /* HAVE_STRUCT_TIMESPEC | _TIMESPEC_DEFINED */
 
@@ -55,38 +48,34 @@ struct timespec {
 #  define ETIMEDOUT 10060     /* This is the value in winsock.h. */
 #endif
 
-#define usbi_mutexattr_t void
-#define usbi_condattr_t  void
-#define usbi_tls_key_t   DWORD
-
-// all Windows mutexes are recursive
-#define usbi_mutex_init_recursive(mutex, attr) usbi_mutex_init((mutex), (attr))
+#define usbi_mutexattr_t	void
+#define usbi_condattr_t		void
+#define usbi_tls_key_t		DWORD
 
 int usbi_mutex_static_lock(usbi_mutex_static_t *mutex);
 int usbi_mutex_static_unlock(usbi_mutex_static_t *mutex);
 
-
-int usbi_mutex_init(usbi_mutex_t *mutex,
-					const usbi_mutexattr_t *attr);
+int usbi_mutex_init(usbi_mutex_t *mutex, const usbi_mutexattr_t *attr);
 int usbi_mutex_lock(usbi_mutex_t *mutex);
 int usbi_mutex_unlock(usbi_mutex_t *mutex);
 int usbi_mutex_trylock(usbi_mutex_t *mutex);
 int usbi_mutex_destroy(usbi_mutex_t *mutex);
 
-int usbi_cond_init(usbi_cond_t *cond,
-				   const usbi_condattr_t *attr);
-int usbi_cond_destroy(usbi_cond_t *cond);
+int usbi_cond_init(usbi_cond_t *cond, const usbi_condattr_t *attr);
 int usbi_cond_wait(usbi_cond_t *cond, usbi_mutex_t *mutex);
 int usbi_cond_timedwait(usbi_cond_t *cond,
-						usbi_mutex_t *mutex,
-						const struct timespec *abstime);
+	usbi_mutex_t *mutex, const struct timespec *abstime);
 int usbi_cond_broadcast(usbi_cond_t *cond);
 int usbi_cond_signal(usbi_cond_t *cond);
+int usbi_cond_destroy(usbi_cond_t *cond);
 
-int   usbi_tls_key_create(usbi_tls_key_t *key, void (*destructor)(void *));
+int usbi_tls_key_create(usbi_tls_key_t *key, void (*destructor)(void *));
 void *usbi_tls_key_get(usbi_tls_key_t key);
-int   usbi_tls_key_set(usbi_tls_key_t key, const void *value);
-int   usbi_tls_key_delete(usbi_tls_key_t key);
+int usbi_tls_key_set(usbi_tls_key_t key, const void *value);
+int usbi_tls_key_delete(usbi_tls_key_t key);
+
+// all Windows mutexes are recursive
+#define usbi_mutex_init_recursive(mutex, attr) usbi_mutex_init((mutex), (attr))
 
 int usbi_get_tid(void);
 
diff --git a/libusb/version_nano.h b/libusb/version_nano.h
index b5eb186..9ea07cd 100644
--- a/libusb/version_nano.h
+++ b/libusb/version_nano.h
@@ -1 +1 @@
-#define LIBUSB_NANO 11048
+#define LIBUSB_NANO 11049