SDL: thread: Locking mutexes and rwlocks are now void functions.

From 899eb0d04216fcef77b0c9f2fad5acfd4d35234c Mon Sep 17 00:00:00 2001
From: "Ryan C. Gordon" <[EMAIL REDACTED]>
Date: Wed, 25 Oct 2023 10:00:26 -0400
Subject: [PATCH] thread: Locking mutexes and rwlocks are now void functions.

Almost nothing checks these return values, and there's no reason a valid
lock should fail to operate. The cases where a lock isn't valid (it's a
bogus pointer, it was previously destroyed, a thread is unlocking a lock it
doesn't own, etc) are undefined behavior and always were, and should be
treated as an application bug.

Reference Issue #8096.
---
 docs/README-migration.md               |   2 +
 include/SDL3/SDL_mutex.h               |  56 +++++----
 src/SDL_properties.c                   |  66 +++++------
 src/audio/SDL_audiocvt.c               |  12 +-
 src/dynapi/SDL_dynapi_procs.h          |  10 +-
 src/thread/generic/SDL_sysmutex.c      | 135 +++++++++-------------
 src/thread/generic/SDL_sysrwlock.c     | 122 +++++++++-----------
 src/thread/generic/SDL_sysrwlock_c.h   |  10 +-
 src/thread/n3ds/SDL_sysmutex.c         |  40 ++-----
 src/thread/ngage/SDL_sysmutex.cpp      |  28 ++---
 src/thread/psp/SDL_sysmutex.c          | 101 ++++++-----------
 src/thread/pthread/SDL_sysmutex.c      | 150 +++++++++++--------------
 src/thread/pthread/SDL_sysrwlock.c     |  53 ++++-----
 src/thread/stdcpp/SDL_sysmutex.cpp     |  59 +++-------
 src/thread/stdcpp/SDL_sysrwlock.cpp    |  90 ++++++---------
 src/thread/vita/SDL_sysmutex.c         |  89 +++++----------
 src/thread/windows/SDL_sysmutex.c      |  81 +++++--------
 src/thread/windows/SDL_sysmutex_c.h    |   4 +-
 src/thread/windows/SDL_sysrwlock_srw.c |  88 ++++++---------
 test/testlock.c                        |  11 +-
 test/testrwlock.c                      |  23 ++--
 21 files changed, 490 insertions(+), 740 deletions(-)

diff --git a/docs/README-migration.md b/docs/README-migration.md
index ae1693de7c9b..aa1b044f1209 100644
--- a/docs/README-migration.md
+++ b/docs/README-migration.md
@@ -707,6 +707,8 @@ The following functions have been renamed:
 
 ## SDL_mutex.h
 
+SDL_LockMutex and SDL_UnlockMutex now return void; if the mutex is valid (including being a NULL pointer, which returns immediately), these functions never fail. If the mutex is invalid or the caller does something illegal, like unlock another thread's mutex, this is considered undefined behavior.
+
 The following functions have been renamed:
 * SDL_CondBroadcast() => SDL_BroadcastCondition()
 * SDL_CondSignal() => SDL_SignalCondition()
diff --git a/include/SDL3/SDL_mutex.h b/include/SDL3/SDL_mutex.h
index 23fb86c30290..7dd150259f67 100644
--- a/include/SDL3/SDL_mutex.h
+++ b/include/SDL3/SDL_mutex.h
@@ -169,13 +169,15 @@ extern DECLSPEC SDL_Mutex *SDLCALL SDL_CreateMutex(void);
  * unlock it the same number of times before it is actually made available for
  * other threads in the system (this is known as a "recursive mutex").
  *
+ * This function does not fail; if mutex is NULL, it will return immediately
+ * having locked nothing. If the mutex is valid, this function will always
+ * block until it can lock the mutex, and return with it locked.
+ *
  * \param mutex the mutex to lock
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
  */
-extern DECLSPEC int SDLCALL SDL_LockMutex(SDL_Mutex *mutex) SDL_ACQUIRE(mutex);
+extern DECLSPEC void SDLCALL SDL_LockMutex(SDL_Mutex *mutex) SDL_ACQUIRE(mutex);
 
 /**
  * Try to lock a mutex without blocking.
@@ -186,9 +188,13 @@ extern DECLSPEC int SDLCALL SDL_LockMutex(SDL_Mutex *mutex) SDL_ACQUIRE(mutex);
  * This technique is useful if you need exclusive access to a resource but
  * don't want to wait for it, and will return to it to try again later.
  *
+ * This function does not fail; if mutex is NULL, it will return 0 immediately
+ * having locked nothing. If the mutex is valid, this function will always
+ * either lock the mutex and return 0, or return SDL_MUTEX_TIMEOUT and lock
+ * nothing.
+ *
  * \param mutex the mutex to try to lock
- * \returns 0, `SDL_MUTEX_TIMEDOUT`, or -1 on error; call SDL_GetError() for
- *          more information.
+ * \returns 0 or `SDL_MUTEX_TIMEDOUT`
  *
  * \since This function is available since SDL 3.0.0.
  *
@@ -210,12 +216,10 @@ extern DECLSPEC int SDLCALL SDL_TryLockMutex(SDL_Mutex *mutex) SDL_TRY_ACQUIRE(0
  * thread, and doing so results in undefined behavior.
  *
  * \param mutex the mutex to unlock.
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
  */
-extern DECLSPEC int SDLCALL SDL_UnlockMutex(SDL_Mutex *mutex) SDL_RELEASE(mutex);
+extern DECLSPEC void SDLCALL SDL_UnlockMutex(SDL_Mutex *mutex) SDL_RELEASE(mutex);
 
 /**
  * Destroy a mutex created with SDL_CreateMutex().
@@ -321,15 +325,17 @@ extern DECLSPEC SDL_RWLock *SDLCALL SDL_CreateRWLock(void);
  * lock before requesting a read-only lock. (But, of course, if you have the
  * write lock, you don't need further locks to read in any case.)
  *
+ * This function does not fail; if rwlock is NULL, it will return immediately
+ * having locked nothing. If the rwlock is valid, this function will always
+ * block until it can lock the mutex, and return with it locked.
+ *
  * \param rwlock the read/write lock to lock
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_UnlockRWLock
  */
-extern DECLSPEC int SDLCALL SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_ACQUIRE_SHARED(rwlock);
+extern DECLSPEC void SDLCALL SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_ACQUIRE_SHARED(rwlock);
 
 /**
  * Lock the read/write lock for _write_ operations.
@@ -348,15 +354,17 @@ extern DECLSPEC int SDLCALL SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_ACQ
  * read-only lock. Doing so results in undefined behavior. Unlock the
  * read-only lock before requesting a write lock.
  *
+ * This function does not fail; if rwlock is NULL, it will return immediately
+ * having locked nothing. If the rwlock is valid, this function will always
+ * block until it can lock the mutex, and return with it locked.
+ *
  * \param rwlock the read/write lock to lock
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
  *
  * \sa SDL_UnlockRWLock
  */
-extern DECLSPEC int SDLCALL SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_ACQUIRE(rwlock);
+extern DECLSPEC void SDLCALL SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_ACQUIRE(rwlock);
 
 /**
  * Try to lock a read/write lock _for reading_ without blocking.
@@ -370,9 +378,13 @@ extern DECLSPEC int SDLCALL SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_ACQ
  * Trying to lock for read-only access can succeed if other threads are
  * holding read-only locks, as this won't prevent access.
  *
+ * This function does not fail; if rwlock is NULL, it will return 0 immediately
+ * having locked nothing. If rwlock is valid, this function will always
+ * either lock the rwlock and return 0, or return SDL_RWLOCK_TIMEOUT and lock
+ * nothing.
+ *
  * \param rwlock the rwlock to try to lock
- * \returns 0, `SDL_RWLOCK_TIMEDOUT`, or -1 on error; call SDL_GetError() for
- *          more information.
+ * \returns 0 or `SDL_RWLOCK_TIMEDOUT`
  *
  * \since This function is available since SDL 3.0.0.
  *
@@ -400,9 +412,13 @@ extern DECLSPEC int SDLCALL SDL_TryLockRWLockForReading(SDL_RWLock *rwlock) SDL_
  * read-only lock. Doing so results in undefined behavior. Unlock the
  * read-only lock before requesting a write lock.
  *
+ * This function does not fail; if rwlock is NULL, it will return 0 immediately
+ * having locked nothing. If rwlock is valid, this function will always
+ * either lock the rwlock and return 0, or return SDL_RWLOCK_TIMEOUT and lock
+ * nothing.
+ *
  * \param rwlock the rwlock to try to lock
- * \returns 0, `SDL_RWLOCK_TIMEDOUT`, or -1 on error; call SDL_GetError() for
- *          more information.
+ * \returns 0 or `SDL_RWLOCK_TIMEDOUT`
  *
  * \since This function is available since SDL 3.0.0.
  *
@@ -428,12 +444,10 @@ extern DECLSPEC int SDLCALL SDL_TryLockRWLockForWriting(SDL_RWLock *rwlock) SDL_
  * thread, and doing so results in undefined behavior.
  *
  * \param rwlock the rwlock to unlock.
- * \returns 0 on success or a negative error code on failure; call
- *          SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
  */
-extern DECLSPEC int SDLCALL SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_RELEASE_GENERIC(rwlock);
+extern DECLSPEC void SDLCALL SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_RELEASE_GENERIC(rwlock);
 
 /**
  * Destroy a read/write lock created with SDL_CreateRWLock().
diff --git a/src/SDL_properties.c b/src/SDL_properties.c
index 22613752d3d9..0b8d79534781 100644
--- a/src/SDL_properties.c
+++ b/src/SDL_properties.c
@@ -123,17 +123,16 @@ SDL_PropertiesID SDL_CreateProperties(void)
         goto error;
     }
 
-    if (SDL_LockRWLockForWriting(SDL_properties_lock) == 0) {
+    SDL_LockRWLockForWriting(SDL_properties_lock);
+    ++SDL_last_properties_id;
+    if (SDL_last_properties_id == 0) {
         ++SDL_last_properties_id;
-        if (SDL_last_properties_id == 0) {
-            ++SDL_last_properties_id;
-        }
-        props = SDL_last_properties_id;
-        if (SDL_InsertIntoHashTable(SDL_properties, (const void *)(uintptr_t)props, properties)) {
-            inserted = SDL_TRUE;
-        }
-        SDL_UnlockRWLock(SDL_properties_lock);
     }
+    props = SDL_last_properties_id;
+    if (SDL_InsertIntoHashTable(SDL_properties, (const void *)(uintptr_t)props, properties)) {
+        inserted = SDL_TRUE;
+    }
+    SDL_UnlockRWLock(SDL_properties_lock);
 
     if (inserted) {
         /* All done! */
@@ -152,15 +151,17 @@ int SDL_LockProperties(SDL_PropertiesID props)
     if (!props) {
         return SDL_InvalidParamError("props");
     }
-    if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
-        SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-        SDL_UnlockRWLock(SDL_properties_lock);
-    }
+
+    SDL_LockRWLockForReading(SDL_properties_lock);
+    SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+    SDL_UnlockRWLock(SDL_properties_lock);
+
     if (!properties) {
-        SDL_InvalidParamError("props");
-        return -1;
+        return SDL_InvalidParamError("props");
     }
-    return SDL_LockMutex(properties->lock);
+
+    SDL_LockMutex(properties->lock);
+    return 0;
 }
 
 void SDL_UnlockProperties(SDL_PropertiesID props)
@@ -170,13 +171,15 @@ void SDL_UnlockProperties(SDL_PropertiesID props)
     if (!props) {
         return;
     }
-    if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
-        SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-        SDL_UnlockRWLock(SDL_properties_lock);
-    }
+
+    SDL_LockRWLockForReading(SDL_properties_lock);
+    SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+    SDL_UnlockRWLock(SDL_properties_lock);
+
     if (!properties) {
         return;
     }
+
     SDL_UnlockMutex(properties->lock);
 }
 
@@ -193,10 +196,10 @@ int SDL_SetProperty(SDL_PropertiesID props, const char *name, void *value, void
         return SDL_InvalidParamError("name");
     }
 
-    if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
-        SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-        SDL_UnlockRWLock(SDL_properties_lock);
-    }
+    SDL_LockRWLockForReading(SDL_properties_lock);
+    SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+    SDL_UnlockRWLock(SDL_properties_lock);
+
     if (!properties) {
         return SDL_InvalidParamError("props");
     }
@@ -242,10 +245,10 @@ void *SDL_GetProperty(SDL_PropertiesID props, const char *name)
         return NULL;
     }
 
-    if (SDL_LockRWLockForReading(SDL_properties_lock) == 0) {
-        SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
-        SDL_UnlockRWLock(SDL_properties_lock);
-    }
+    SDL_LockRWLockForReading(SDL_properties_lock);
+    SDL_FindInHashTable(SDL_properties, (const void *)(uintptr_t)props, (const void **)&properties);
+    SDL_UnlockRWLock(SDL_properties_lock);
+
     if (!properties) {
         SDL_InvalidParamError("props");
         return NULL;
@@ -280,8 +283,7 @@ void SDL_DestroyProperties(SDL_PropertiesID props)
         return;
     }
 
-    if (SDL_LockRWLockForWriting(SDL_properties_lock) == 0) {
-        SDL_RemoveFromHashTable(SDL_properties, (const void *)(uintptr_t)props);
-        SDL_UnlockRWLock(SDL_properties_lock);
-    }
+    SDL_LockRWLockForWriting(SDL_properties_lock);
+    SDL_RemoveFromHashTable(SDL_properties, (const void *)(uintptr_t)props);
+    SDL_UnlockRWLock(SDL_properties_lock);
 }
diff --git a/src/audio/SDL_audiocvt.c b/src/audio/SDL_audiocvt.c
index 7681e76cb9cd..b64ca480dc84 100644
--- a/src/audio/SDL_audiocvt.c
+++ b/src/audio/SDL_audiocvt.c
@@ -476,12 +476,20 @@ int SDL_SetAudioStreamPutCallback(SDL_AudioStream *stream, SDL_AudioStreamCallba
 
 int SDL_LockAudioStream(SDL_AudioStream *stream)
 {
-    return stream ? SDL_LockMutex(stream->lock) : SDL_InvalidParamError("stream");
+    if (!stream) {
+        return SDL_InvalidParamError("stream");
+    }
+    SDL_LockMutex(stream->lock);
+    return 0;
 }
 
 int SDL_UnlockAudioStream(SDL_AudioStream *stream)
 {
-    return stream ? SDL_UnlockMutex(stream->lock) : SDL_InvalidParamError("stream");
+    if (!stream) {
+        return SDL_InvalidParamError("stream");
+    }
+    SDL_UnlockMutex(stream->lock);
+    return 0;
 }
 
 int SDL_GetAudioStreamFormat(SDL_AudioStream *stream, SDL_AudioSpec *src_spec, SDL_AudioSpec *dst_spec)
diff --git a/src/dynapi/SDL_dynapi_procs.h b/src/dynapi/SDL_dynapi_procs.h
index b8ac62c583b0..ac41615e9de1 100644
--- a/src/dynapi/SDL_dynapi_procs.h
+++ b/src/dynapi/SDL_dynapi_procs.h
@@ -519,9 +519,9 @@ SDL_DYNAPI_PROC(void*,SDL_LoadFile_RW,(SDL_RWops *a, size_t *b, SDL_bool c),(a,b
 SDL_DYNAPI_PROC(SDL_FunctionPointer,SDL_LoadFunction,(void *a, const char *b),(a,b),return)
 SDL_DYNAPI_PROC(void*,SDL_LoadObject,(const char *a),(a),return)
 SDL_DYNAPI_PROC(void,SDL_LockJoysticks,(void),(),)
-SDL_DYNAPI_PROC(int,SDL_LockMutex,(SDL_Mutex *a),(a),return)
-SDL_DYNAPI_PROC(int,SDL_LockRWLockForReading,(SDL_RWLock *a),(a),return)
-SDL_DYNAPI_PROC(int,SDL_LockRWLockForWriting,(SDL_RWLock *a),(a),return)
+SDL_DYNAPI_PROC(void,SDL_LockMutex,(SDL_Mutex *a),(a),)
+SDL_DYNAPI_PROC(void,SDL_LockRWLockForReading,(SDL_RWLock *a),(a),)
+SDL_DYNAPI_PROC(void,SDL_LockRWLockForWriting,(SDL_RWLock *a),(a),)
 SDL_DYNAPI_PROC(int,SDL_LockSurface,(SDL_Surface *a),(a),return)
 SDL_DYNAPI_PROC(int,SDL_LockTexture,(SDL_Texture *a, const SDL_Rect *b, void **c, int *d),(a,b,c,d),return)
 SDL_DYNAPI_PROC(int,SDL_LockTextureToSurface,(SDL_Texture *a, const SDL_Rect *b, SDL_Surface **c),(a,b,c),return)
@@ -706,8 +706,8 @@ SDL_DYNAPI_PROC(int,SDL_TryLockRWLockForWriting,(SDL_RWLock *a),(a),return)
 SDL_DYNAPI_PROC(int,SDL_TryWaitSemaphore,(SDL_Semaphore *a),(a),return)
 SDL_DYNAPI_PROC(void,SDL_UnloadObject,(void *a),(a),)
 SDL_DYNAPI_PROC(void,SDL_UnlockJoysticks,(void),(),)
-SDL_DYNAPI_PROC(int,SDL_UnlockMutex,(SDL_Mutex *a),(a),return)
-SDL_DYNAPI_PROC(int,SDL_UnlockRWLock,(SDL_RWLock *a),(a),return)
+SDL_DYNAPI_PROC(void,SDL_UnlockMutex,(SDL_Mutex *a),(a),)
+SDL_DYNAPI_PROC(void,SDL_UnlockRWLock,(SDL_RWLock *a),(a),)
 SDL_DYNAPI_PROC(void,SDL_UnlockSurface,(SDL_Surface *a),(a),)
 SDL_DYNAPI_PROC(void,SDL_UnlockTexture,(SDL_Texture *a),(a),)
 SDL_DYNAPI_PROC(void,SDL_UpdateGamepads,(void),(),)
diff --git a/src/thread/generic/SDL_sysmutex.c b/src/thread/generic/SDL_sysmutex.c
index 8d4a3702d1eb..579d028ee3ef 100644
--- a/src/thread/generic/SDL_sysmutex.c
+++ b/src/thread/generic/SDL_sysmutex.c
@@ -20,7 +20,7 @@
 */
 #include "SDL_internal.h"
 
-/* An implementation of mutexes using semaphores */
+// An implementation of mutexes using semaphores
 
 #include "SDL_systhread_c.h"
 
@@ -31,13 +31,9 @@ struct SDL_Mutex
     SDL_Semaphore *sem;
 };
 
-/* Create a mutex */
 SDL_Mutex *SDL_CreateMutex(void)
 {
-    SDL_Mutex *mutex;
-
-    /* Allocate mutex memory */
-    mutex = (SDL_Mutex *)SDL_calloc(1, sizeof(*mutex));
+    SDL_Mutex *mutex = (SDL_Mutex *)SDL_calloc(1, sizeof(*mutex));
 
 #ifndef SDL_THREADS_DISABLED
     if (mutex) {
@@ -52,12 +48,11 @@ SDL_Mutex *SDL_CreateMutex(void)
     } else {
         SDL_OutOfMemory();
     }
-#endif /* !SDL_THREADS_DISABLED */
+#endif // !SDL_THREADS_DISABLED
 
     return mutex;
 }
 
-/* Free the mutex */
 void SDL_DestroyMutex(SDL_Mutex *mutex)
 {
     if (mutex) {
@@ -68,94 +63,72 @@ void SDL_DestroyMutex(SDL_Mutex *mutex)
     }
 }
 
-/* Lock the mutex */
-int SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS  // clang doesn't know about NULL mutexes
 {
-#ifdef SDL_THREADS_DISABLED
-    return 0;
-#else
-    SDL_threadID this_thread;
-
-    if (mutex == NULL) {
-        return 0;
-    }
-
-    this_thread = SDL_ThreadID();
-    if (mutex->owner == this_thread) {
-        ++mutex->recursive;
-    } else {
-        /* The order of operations is important.
-           We set the locking thread id after we obtain the lock
-           so unlocks from other threads will fail.
-         */
-        SDL_WaitSemaphore(mutex->sem);
-        mutex->owner = this_thread;
-        mutex->recursive = 0;
+#ifndef SDL_THREADS_DISABLED
+    if (mutex != NULL) {
+        SDL_threadID this_thread = SDL_ThreadID();
+        if (mutex->owner == this_thread) {
+            ++mutex->recursive;
+        } else {
+            /* The order of operations is important.
+               We set the locking thread id after we obtain the lock
+               so unlocks from other threads will fail.
+             */
+            SDL_WaitSemaphore(mutex->sem);
+            mutex->owner = this_thread;
+            mutex->recursive = 0;
+        }
     }
-
-    return 0;
 #endif /* SDL_THREADS_DISABLED */
 }
 
-/* try Lock the mutex */
 int SDL_TryLockMutex(SDL_Mutex *mutex)
 {
-#ifdef SDL_THREADS_DISABLED
-    return 0;
-#else
     int retval = 0;
-    SDL_threadID this_thread;
-
-    if (mutex == NULL) {
-        return 0;
-    }
-
-    this_thread = SDL_ThreadID();
-    if (mutex->owner == this_thread) {
-        ++mutex->recursive;
-    } else {
-        /* The order of operations is important.
-         We set the locking thread id after we obtain the lock
-         so unlocks from other threads will fail.
-         */
-        retval = SDL_TryWaitSemaphore(mutex->sem);
-        if (retval == 0) {
-            mutex->owner = this_thread;
-            mutex->recursive = 0;
+#ifndef SDL_THREADS_DISABLED
+    if (mutex != NULL) {
+        SDL_threadID this_thread = SDL_ThreadID();
+        if (mutex->owner == this_thread) {
+            ++mutex->recursive;
+        } else {
+            /* The order of operations is important.
+             We set the locking thread id after we obtain the lock
+             so unlocks from other threads will fail.
+             */
+            retval = SDL_TryWaitSemaphore(mutex->sem);
+            if (retval == 0) {
+                mutex->owner = this_thread;
+                mutex->recursive = 0;
+            }
         }
     }
-
+#endif // SDL_THREADS_DISABLED
     return retval;
-#endif /* SDL_THREADS_DISABLED */
 }
 
-/* Unlock the mutex */
-int SDL_UnlockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_UnlockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS  // clang doesn't know about NULL mutexes
 {
-#ifdef SDL_THREADS_DISABLED
-    return 0;
-#else
-    if (mutex == NULL) {
-        return 0;
-    }
-
-    /* If we don't own the mutex, we can't unlock it */
-    if (SDL_ThreadID() != mutex->owner) {
-        return SDL_SetError("mutex not owned by this thread");
-    }
+#ifndef SDL_THREADS_DISABLED
+    if (mutex != NULL) {
+        // If we don't own the mutex, we can't unlock it
+        if (SDL_ThreadID() != mutex->owner) {
+            SDL_assert(!"Tried to unlock a mutex we don't own!");
+            return; // (undefined behavior!) SDL_SetError("mutex not owned by this thread");
+        }
 
-    if (mutex->recursive) {
-        --mutex->recursive;
-    } else {
-        /* The order of operations is important.
-           First reset the owner so another thread doesn't lock
-           the mutex and set the ownership before we reset it,
-           then release the lock semaphore.
-         */
-        mutex->owner = 0;
-        SDL_PostSemaphore(mutex->sem);
+        if (mutex->recursive) {
+            --mutex->recursive;
+        } else {
+            /* The order of operations is important.
+               First reset the owner so another thread doesn't lock
+               the mutex and set the ownership before we reset it,
+               then release the lock semaphore.
+             */
+            mutex->owner = 0;
+            SDL_PostSemaphore(mutex->sem);
+        }
     }
-    return 0;
-#endif /* SDL_THREADS_DISABLED */
+#endif // SDL_THREADS_DISABLED
 }
 
diff --git a/src/thread/generic/SDL_sysrwlock.c b/src/thread/generic/SDL_sysrwlock.c
index d88bee15d380..a38b81d6ac76 100644
--- a/src/thread/generic/SDL_sysrwlock.c
+++ b/src/thread/generic/SDL_sysrwlock.c
@@ -20,7 +20,7 @@
 */
 #include "SDL_internal.h"
 
-/* An implementation of rwlocks using mutexes, condition variables, and atomics. */
+// An implementation of rwlocks using mutexes, condition variables, and atomics.
 
 #include "SDL_systhread_c.h"
 
@@ -30,7 +30,7 @@
  * will be chosen at runtime), the function names need to be
  * suffixed
  */
-/* !!! FIXME: this is quite a tapdance with macros and the build system, maybe we can simplify how we do this. --ryan. */
+// !!! FIXME: this is quite a tapdance with macros and the build system, maybe we can simplify how we do this. --ryan.
 #ifndef SDL_THREAD_GENERIC_RWLOCK_SUFFIX
 #define SDL_CreateRWLock_generic SDL_CreateRWLock
 #define SDL_DestroyRWLock_generic SDL_DestroyRWLock
@@ -95,63 +95,48 @@ void SDL_DestroyRWLock_generic(SDL_RWLock *rwlock)
     }
 }
 
-int SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS  // clang doesn't know about NULL mutexes
 {
 #ifndef SDL_THREADS_DISABLED
-    if (!rwlock) {
-        return SDL_InvalidParamError("rwlock");
-    } else if (SDL_LockMutex(rwlock->lock) == -1) {
-        return -1;
+    if (rwlock) {
+        // !!! FIXME: these don't have to be atomic, we always gate them behind a mutex.
+        SDL_LockMutex(rwlock->lock);
+        SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0);  // shouldn't be able to grab lock if there's a writer!
+        SDL_AtomicAdd(&rwlock->reader_count, 1);
+        SDL_UnlockMutex(rwlock->lock);   // other readers can attempt to share the lock.
     }
-
-    SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0);  /* shouldn't be able to grab lock if there's a writer! */
-
-    SDL_AtomicAdd(&rwlock->reader_count, 1);
-    SDL_UnlockMutex(rwlock->lock);   /* other readers can attempt to share the lock. */
 #endif
-
-    return 0;
 }
 
-int SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS  // clang doesn't know about NULL mutexes
 {
 #ifndef SDL_THREADS_DISABLED
-    if (!rwlock) {
-        return SDL_InvalidParamError("rwlock");
-    } else if (SDL_LockMutex(rwlock->lock) == -1) {
-        return -1;
-    }
+    if (rwlock) {
+        SDL_LockMutex(rwlock->lock);
+        while (SDL_AtomicGet(&rwlock->reader_count) > 0) {  // while something is holding the shared lock, keep waiting.
+            SDL_WaitCondition(rwlock->condition, rwlock->lock);  // release the lock and wait for readers holding the shared lock to release it, regrab the lock.
+        }
 
-    while (SDL_AtomicGet(&rwlock->reader_count) > 0) {  /* while something is holding the shared lock, keep waiting. */
-        SDL_WaitCondition(rwlock->condition, rwlock->lock);  /* release the lock and wait for readers holding the shared lock to release it, regrab the lock. */
+        // we hold the lock!
+        SDL_AtomicAdd(&rwlock->writer_count, 1);  // we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly!
     }
-
-    /* we hold the lock! */
-    SDL_AtomicAdd(&rwlock->writer_count, 1);  /* we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly! */
 #endif
-
-    return 0;
 }
 
 int SDL_TryLockRWLockForReading_generic(SDL_RWLock *rwlock)
 {
 #ifndef SDL_THREADS_DISABLED
-    int rc;
-
-    if (!rwlock) {
-        return SDL_InvalidParamError("rwlock");
-    }
-
-    rc = SDL_TryLockMutex(rwlock->lock);
-    if (rc != 0) {
-        /* !!! FIXME: there is a small window where a reader has to lock the mutex, and if we hit that, we will return SDL_RWLOCK_TIMEDOUT even though we could have shared the lock. */
-        return rc;
+    if (rwlock) {
+        const int rc = SDL_TryLockMutex(rwlock->lock);
+        if (rc != 0) {
+            // !!! FIXME: there is a small window where a reader has to lock the mutex, and if we hit that, we will return SDL_RWLOCK_TIMEDOUT even though we could have shared the lock.
+            return rc;
+        }
+
+        SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0);  // shouldn't be able to grab lock if there's a writer!
+        SDL_AtomicAdd(&rwlock->reader_count, 1);
+        SDL_UnlockMutex(rwlock->lock);   // other readers can attempt to share the lock.
     }
-
-    SDL_assert(SDL_AtomicGet(&rwlock->writer_count) == 0);  /* shouldn't be able to grab lock if there's a writer! */
-
-    SDL_AtomicAdd(&rwlock->reader_count, 1);
-    SDL_UnlockMutex(rwlock->lock);   /* other readers can attempt to share the lock. */
 #endif
 
     return 0;
@@ -160,46 +145,41 @@ int SDL_TryLockRWLockForReading_generic(SDL_RWLock *rwlock)
 int SDL_TryLockRWLockForWriting_generic(SDL_RWLock *rwlock)
 {
 #ifndef SDL_THREADS_DISABLED
-    int rc;
-
-    if (!rwlock) {
-        return SDL_InvalidParamError("rwlock");
-    } else if ((rc = SDL_TryLockMutex(rwlock->lock)) != 0) {
-        return rc;
-    }
-
-    if (SDL_AtomicGet(&rwlock->reader_count) > 0) {  /* a reader is using the shared lock, treat it as unavailable. */
-        SDL_UnlockMutex(rwlock->lock);
-        return SDL_RWLOCK_TIMEDOUT;
+    if (rwlock) {
+        const int rc = SDL_TryLockMutex(rwlock->lock);
+        if (rc != 0) {
+            return rc;
+        }
+
+        if (SDL_AtomicGet(&rwlock->reader_count) > 0) {  // a reader is using the shared lock, treat it as unavailable.
+            SDL_UnlockMutex(rwlock->lock);
+            return SDL_RWLOCK_TIMEDOUT;
+        }
+
+        // we hold the lock!
+        SDL_AtomicAdd(&rwlock->writer_count, 1);  // we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly!
     }
-
-    /* we hold the lock! */
-    SDL_AtomicAdd(&rwlock->writer_count, 1);  /* we let these be recursive, but the API doesn't require this. It _does_ trust you unlock correctly! */
 #endif
 
     return 0;
 }
 
-int SDL_UnlockRWLock_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
+void SDL_UnlockRWLock_generic(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS  // clang doesn't know about NULL mutexes
 {
 #ifndef SDL_THREADS_DISABLED
-    if (!rwlock) {
-        return SDL_InvalidParamError("rwlock");
-    }
+    if (rwlock) {
+        SDL_LockMutex(rwlock->lock);  // recursive lock for writers, readers grab lock to make sure things are sane.
 
-    SDL_LockMutex(rwlock->lock);  /* recursive lock for writers, readers grab lock to make sure things are sane. */
+        if (SDL_AtomicGet(&rwlock->reader_count) > 0) {  // we're a reader
+            SDL_AtomicAdd(&rwlock->reader_count, -1);
+            SDL_BroadcastCondition(rwlock->condition);  // alert any pending writers to attempt to try to grab the lock again.
+        } else if (SDL_AtomicGet(&rwlock->writer_count) > 0) {  // we're a writer
+            SDL_AtomicAdd(&rwlock->writer_count, -1);
+            SDL_UnlockMutex(rwlock->lock);  // recursive unlock.
+        }
 
-    if (SDL_AtomicGet(&rwlock->reader_count) > 0) {  /* we're a reader */
-        SDL_AtomicAdd(&rwlock->reader_count, -1);
-        SDL_BroadcastCondition(rwlock->condition);  /* alert any pending writers to attempt to try to grab the lock again. */
-    } else if (SDL_AtomicGet(&rwlock->writer_count) > 0) {  /* we're a writer */
-        SDL_AtomicAdd(&rwlock->writer_count, -1);
-        SDL_UnlockMutex(rwlock->lock);  /* recursive unlock. */
+        SDL_UnlockMutex(rwlock->lock);
     }
-
-    SDL_UnlockMutex(rwlock->lock);
 #endif
-
-    return 0;
 }
 
diff --git a/src/thread/generic/SDL_sysrwlock_c.h b/src/thread/generic/SDL_sysrwlock_c.h
index 8247f8ebe684..9083cc89c84e 100644
--- a/src/thread/generic/SDL_sysrwlock_c.h
+++ b/src/thread/generic/SDL_sysrwlock_c.h
@@ -27,12 +27,12 @@
 
 SDL_RWLock *SDL_CreateRWLock_generic(void);
 void SDL_DestroyRWLock_generic(SDL_RWLock *rwlock);
-int SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock);
-int SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock);
+void SDL_LockRWLockForReading_generic(SDL_RWLock *rwlock);
+void  SDL_LockRWLockForWriting_generic(SDL_RWLock *rwlock);
 int SDL_TryLockRWLockForReading_generic(SDL_RWLock *rwlock);
 int SDL_TryLockRWLockForWriting_generic(SDL_RWLock *rwlock);
-int SDL_UnlockRWLock_generic(SDL_RWLock *rwlock);
+void SDL_UnlockRWLock_generic(SDL_RWLock *rwlock);
 
-#endif /* SDL_THREAD_GENERIC_RWLOCK_SUFFIX */
+#endif // SDL_THREAD_GENERIC_RWLOCK_SUFFIX
 
-#endif /* SDL_sysrwlock_c_h_ */
+#endif // SDL_sysrwlock_c_h_
diff --git a/src/thread/n3ds/SDL_sysmutex.c b/src/thread/n3ds/SDL_sysmutex.c
index 0995ad11f4a3..47e848582da2 100644
--- a/src/thread/n3ds/SDL_sysmutex.c
+++ b/src/thread/n3ds/SDL_sysmutex.c
@@ -22,17 +22,13 @@
 
 #ifdef SDL_THREAD_N3DS
 
-/* An implementation of mutexes using libctru's RecursiveLock */
+// An implementation of mutexes using libctru's RecursiveLock
 
 #include "SDL_sysmutex_c.h"
 
-/* Create a mutex */
 SDL_Mutex *SDL_CreateMutex(void)
 {
-    SDL_Mutex *mutex;
-
-    /* Allocate mutex memory */
-    mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex));
+    SDL_Mutex *mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex));
     if (mutex) {
         RecursiveLock_Init(&mutex->lock);
     } else {
@@ -41,7 +37,6 @@ SDL_Mutex *SDL_CreateMutex(void)
     return mutex;
 }
 
-/* Free the mutex */
 void SDL_DestroyMutex(SDL_Mutex *mutex)
 {
     if (mutex) {
@@ -49,38 +44,23 @@ void SDL_DestroyMutex(SDL_Mutex *mutex)
     }
 }
 
-/* Lock the mutex */
-int SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSI

(Patch may be truncated, please check the link at the top of this post.)