Fix or remove non-portable features in thread API

Thread function return values are inconsistent between nearly every threading
API out there.  So, just ignore them entirely, and provide a typedef and
sentinel value so user code can be portable.

6 files changed, 98 insertions, 30 deletions

diff --git a/.clang-format b/.clang-format
index eabae64..f03272e 100644
--- a/.clang-format
+++ b/.clang-format
@@ -24,6 +24,7 @@ StatementMacros:
   - ZIX_CONST_API
   - ZIX_MALLOC_API
   - ZIX_PURE_API
+  - ZIX_THREAD_FUNC
   - ZIX_UNUSED
   - _Pragma
 ...
diff --git a/include/zix/thread.h b/include/zix/thread.h
index c80add0..29dee44 100644
--- a/include/zix/thread.h
+++ b/include/zix/thread.h
@@ -26,47 +26,68 @@ extern "C" {
 */
 
 #ifdef _WIN32
+#  define ZIX_THREAD_RESULT 0
+#  define ZIX_THREAD_FUNC __attribute__((stdcall))
+
 typedef HANDLE ZixThread;
+typedef DWORD  ZixThreadResult;
+
 #else
+#  define ZIX_THREAD_RESULT NULL
+#  define ZIX_THREAD_FUNC
+
 typedef pthread_t ZixThread;
+typedef void*     ZixThreadResult;
 #endif
 
 /**
+   A thread function.
+
+   For portability reasons, the return type varies between platforms, and the
+   return value is ignored.  Thread functions should always return
+   #ZIX_THREAD_RESULT.  This allows thread functions to be used directly by the
+   system without any wrapper overhead.
+
+   "Returning" a result, and communicating with the parent thread in general,
+   can be done through the pointer argument.
+*/
+typedef ZIX_THREAD_FUNC
+ZixThreadResult (*ZixThreadFunc)(void*);
+
+/**
    Initialize `thread` to a new thread.
 
    The thread will immediately be launched, calling `function` with `arg`
    as the only parameter.
 */
 static inline ZixStatus
-zix_thread_create(ZixThread* thread,
-                  size_t     stack_size,
-                  void* (*function)(void*),
-                  void* arg);
+zix_thread_create(ZixThread*    thread,
+                  size_t        stack_size,
+                  ZixThreadFunc function,
+                  void*         arg);
 
 /// Join `thread` (block until `thread` exits)
 static inline ZixStatus
-zix_thread_join(ZixThread thread, void** retval);
+zix_thread_join(ZixThread thread);
 
 #ifdef _WIN32
 
 static inline ZixStatus
-zix_thread_create(ZixThread* thread,
-                  size_t     stack_size,
-                  void* (*function)(void*),
-                  void* arg)
+zix_thread_create(ZixThread*    thread,
+                  size_t        stack_size,
+                  ZixThreadFunc function,
+                  void*         arg)
 {
-  *thread = CreateThread(
-    NULL, stack_size, (LPTHREAD_START_ROUTINE)function, arg, 0, NULL);
+  *thread = CreateThread(NULL, stack_size, function, arg, 0, NULL);
   return *thread ? ZIX_STATUS_SUCCESS : ZIX_STATUS_ERROR;
 }
 
 static inline ZixStatus
-zix_thread_join(ZixThread thread, void** retval)
+zix_thread_join(ZixThread thread)
 {
-  (void)retval;
-
-  return WaitForSingleObject(thread, INFINITE) ? ZIX_STATUS_SUCCESS
-                                               : ZIX_STATUS_ERROR;
+  return (WaitForSingleObject(thread, INFINITE) == WAIT_OBJECT_0)
+           ? ZIX_STATUS_SUCCESS
+           : ZIX_STATUS_ERROR;
 }
 
 #else /* !defined(_WIN32) */
@@ -88,9 +109,9 @@ zix_thread_create(ZixThread* thread,
 }
 
 static inline ZixStatus
-zix_thread_join(ZixThread thread, void** retval)
+zix_thread_join(ZixThread thread)
 {
-  return pthread_join(thread, retval) ? ZIX_STATUS_ERROR : ZIX_STATUS_SUCCESS;
+  return pthread_join(thread, NULL) ? ZIX_STATUS_ERROR : ZIX_STATUS_SUCCESS;
 }
 
 #endif
diff --git a/meson.build b/meson.build
index 2a31887..2f10fbb 100644
--- a/meson.build
+++ b/meson.build
@@ -184,6 +184,7 @@ sequential_tests = [
 threaded_tests = [
   'test_ring',
   'test_sem',
+  'test_thread',
 ]
 
 if not get_option('tests').disabled() and not meson.is_subproject()
diff --git a/test/test_ring.c b/test/test_ring.c
index 79aa0f1..8016910 100644
--- a/test/test_ring.c
+++ b/test/test_ring.c
@@ -43,7 +43,8 @@ cmp_msg(const int* const msg1, const int* const msg2)
   return 1;
 }
 
-static void*
+ZIX_THREAD_FUNC
+static ZixThreadResult
 reader(void* ZIX_UNUSED(arg))
 {
   printf("Reader starting\n");
@@ -63,10 +64,11 @@ reader(void* ZIX_UNUSED(arg))
   }
 
   printf("Reader finished\n");
-  return NULL;
+  return ZIX_THREAD_RESULT;
 }
 
-static void*
+ZIX_THREAD_FUNC
+static ZixThreadResult
 writer(void* ZIX_UNUSED(arg))
 {
   printf("Writer starting\n");
@@ -82,7 +84,7 @@ writer(void* ZIX_UNUSED(arg))
   }
 
   printf("Writer finished\n");
-  return NULL;
+  return ZIX_THREAD_RESULT;
 }
 
 static int
@@ -108,8 +110,8 @@ test_ring(const unsigned size)
   ZixThread writer_thread; // NOLINT
   assert(!zix_thread_create(&writer_thread, MSG_SIZE * 4UL, writer, NULL));
 
-  zix_thread_join(reader_thread, NULL);
-  zix_thread_join(writer_thread, NULL);
+  assert(!zix_thread_join(reader_thread));
+  assert(!zix_thread_join(writer_thread));
 
   assert(!read_error);
   assert(ring);
diff --git a/test/test_sem.c b/test/test_sem.c
index 33859f1..bad0a44 100644
--- a/test/test_sem.c
+++ b/test/test_sem.c
@@ -14,7 +14,8 @@
 static ZixSem   sem;
 static unsigned n_signals = 1024;
 
-static void*
+ZIX_THREAD_FUNC
+static ZixThreadResult
 reader(void* ZIX_UNUSED(arg))
 {
   printf("Reader starting\n");
@@ -24,10 +25,11 @@ reader(void* ZIX_UNUSED(arg))
   }
 
   printf("Reader finished\n");
-  return NULL;
+  return ZIX_THREAD_RESULT;
 }
 
-static void*
+ZIX_THREAD_FUNC
+static ZixThreadResult
 writer(void* ZIX_UNUSED(arg))
 {
   printf("Writer starting\n");
@@ -37,7 +39,7 @@ writer(void* ZIX_UNUSED(arg))
   }
 
   printf("Writer finished\n");
-  return NULL;
+  return ZIX_THREAD_RESULT;
 }
 
 int
@@ -62,8 +64,8 @@ main(int argc, char** argv)
   ZixThread writer_thread; // NOLINT
   assert(!zix_thread_create(&writer_thread, 128, writer, NULL));
 
-  zix_thread_join(reader_thread, NULL);
-  zix_thread_join(writer_thread, NULL);
+  assert(!zix_thread_join(reader_thread));
+  assert(!zix_thread_join(writer_thread));
 
   zix_sem_destroy(&sem);
   return 0;
diff --git a/test/test_thread.c b/test/test_thread.c
new file mode 100644
index 0000000..6fe80a5
--- /dev/null
+++ b/test/test_thread.c
@@ -0,0 +1,41 @@
+// Copyright 2012-2022 David Robillard <d@drobilla.net>
+// SPDX-License-Identifier: ISC
+
+#undef NDEBUG
+
+#include "zix/thread.h"
+
+#include <assert.h>
+#include <string.h>
+
+typedef struct {
+  int input;
+  int output;
+} SharedData;
+
+ZIX_THREAD_FUNC
+static ZixThreadResult
+thread_func(void* const arg)
+{
+  SharedData* const data = (SharedData*)arg;
+
+  data->output = data->input * 7;
+
+  return ZIX_THREAD_RESULT;
+}
+
+int
+main(int argc, char** argv)
+{
+  (void)argv;
+
+  ZixThread thread; // NOLINT
+
+  SharedData data = {argc + (int)strlen(argv[0]), 0};
+
+  assert(!zix_thread_create(&thread, 128, thread_func, &data));
+  assert(!zix_thread_join(thread));
+  assert(data.output == data.input * 7);
+
+  return 0;
+}