diff --git a/docs/README-migration.md b/docs/README-migration.md index ae1693de7c..aa1b044f12 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 23fb86c302..7dd150259f 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 22613752d3..0b8d795347 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 7681e76cb9..b64ca480dc 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 b8ac62c583..ac41615e9d 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 8d4a3702d1..579d028ee3 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; - } - - 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) { +#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 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 */ +int SDL_TryLockMutex(SDL_Mutex *mutex) { -#ifdef SDL_THREADS_DISABLED - return 0; -#else - if (mutex == NULL) { - return 0; + int retval = 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; + } + } } - - /* 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"); - } - - 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 + return retval; +} + +void SDL_UnlockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes +{ +#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); + } + } +#endif // SDL_THREADS_DISABLED } diff --git a/src/thread/generic/SDL_sysrwlock.c b/src/thread/generic/SDL_sysrwlock.c index d88bee15d3..a38b81d6ac 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) { + 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; + } - if (!rwlock) { - return SDL_InvalidParamError("rwlock"); + 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. } - - 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. */ #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) { + const int rc = SDL_TryLockMutex(rwlock->lock); + if (rc != 0) { + return 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; + } + + // 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! } - - 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! */ #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. + + 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_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. */ - } - - 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 8247f8ebe6..9083cc89c8 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 0995ad11f4..47e848582d 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_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 { - if (mutex == NULL) { - return 0; + if (mutex != NULL) { + RecursiveLock_Lock(&mutex->lock); } - - RecursiveLock_Lock(&mutex->lock); - - return 0; } -/* try Lock the mutex */ int SDL_TryLockMutex(SDL_Mutex *mutex) { - if (mutex == NULL) { - return 0; - } - - return RecursiveLock_TryLock(&mutex->lock); + return (mutex == NULL) ? 0 : RecursiveLock_TryLock(&mutex->lock); } -/* 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 { - if (mutex == NULL) { - return 0; + if (mutex != NULL) { + RecursiveLock_Unlock(&mutex->lock); } - - RecursiveLock_Unlock(&mutex->lock); - - return 0; } -#endif /* SDL_THREAD_N3DS */ +#endif // SDL_THREAD_N3DS diff --git a/src/thread/ngage/SDL_sysmutex.cpp b/src/thread/ngage/SDL_sysmutex.cpp index 9cc47d424d..4d5e3b08cd 100644 --- a/src/thread/ngage/SDL_sysmutex.cpp +++ b/src/thread/ngage/SDL_sysmutex.cpp @@ -67,17 +67,13 @@ 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 */ { - if (mutex == NULL) { - return 0; + if (mutex != NULL) { + RMutex rmutex; + rmutex.SetHandle(mutex->handle); + rmutex.Wait(); } - - RMutex rmutex; - rmutex.SetHandle(mutex->handle); - rmutex.Wait(); - - return 0; } /* Try to lock the mutex */ @@ -95,16 +91,12 @@ int SDL_TryLockMutex(SDL_Mutex *mutex) #endif /* 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 */ { - if (mutex == NULL) { - return 0; + if (mutex != NULL) { + RMutex rmutex; + rmutex.SetHandle(mutex->handle); + rmutex.Signal(); } - - RMutex rmutex; - rmutex.SetHandle(mutex->handle); - rmutex.Signal(); - - return 0; } diff --git a/src/thread/psp/SDL_sysmutex.c b/src/thread/psp/SDL_sysmutex.c index 8aaa96224d..f6701bb123 100644 --- a/src/thread/psp/SDL_sysmutex.c +++ b/src/thread/psp/SDL_sysmutex.c @@ -22,7 +22,7 @@ #ifdef SDL_THREAD_PSP -/* An implementation of mutexes using semaphores */ +// An implementation of mutexes using semaphores #include "SDL_systhread_c.h" @@ -36,17 +36,11 @@ struct SDL_Mutex SceLwMutexWorkarea lock; }; -/* Create a mutex */ SDL_Mutex *SDL_CreateMutex(void) { - SDL_Mutex *mutex = NULL; - SceInt32 res = 0; - - /* Allocate mutex memory */ - mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex)); + SDL_Mutex *mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex)); if (mutex) { - - res = sceKernelCreateLwMutex( + const SceInt32 res = sceKernelCreateLwMutex( &mutex->lock, "SDL mutex", SCE_KERNEL_MUTEX_ATTR_RECURSIVE, @@ -54,6 +48,8 @@ SDL_Mutex *SDL_CreateMutex(void) NULL); if (res < 0) { + SDL_free(mutex); + mutex = NULL; SDL_SetError("Error trying to create mutex: %lx", res); } } else { @@ -62,7 +58,6 @@ SDL_Mutex *SDL_CreateMutex(void) return mutex; } -/* Free the mutex */ void SDL_DestroyMutex(SDL_Mutex *mutex) { if (mutex) { @@ -71,75 +66,43 @@ 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 - SceInt32 res = 0; - - if (mutex == NULL) { - return 0; +#ifndef SDL_THREADS_DISABLED + if (mutex != NULL) { + const SceInt32 res = sceKernelLockLwMutex(&mutex->lock, 1, NULL); + SDL_assert(res == SCE_KERNEL_ERROR_OK); // assume we're in a lot of trouble if this assert fails. } - - res = sceKernelLockLwMutex(&mutex->lock, 1, NULL); - if (res != SCE_KERNEL_ERROR_OK) { - return SDL_SetError("Error trying to lock mutex: %lx", res); - } - - return 0; -#endif /* SDL_THREADS_DISABLED */ +#endif // SDL_THREADS_DISABLED } -/* Try to lock the mutex */ int SDL_TryLockMutex(SDL_Mutex *mutex) { -#ifdef SDL_THREADS_DISABLED - return 0; -#else - SceInt32 res = 0; - - if (mutex == NULL) { - return 0; + int retval = 0; +#ifndef SDL_THREADS_DISABLED + if (mutex != NULL) { + const SceInt32 res = sceKernelTryLockLwMutex(&mutex->lock, 1); + if (res == SCE_KERNEL_ERROR_OK) { + retval = 0; + } else if (res == SCE_KERNEL_ERROR_WAIT_TIMEOUT) { + retval = SDL_MUTEX_TIMEDOUT; + } else { + SDL_assert(!"Error trying to lock mutex"); // assume we're in a lot of trouble if this assert fails. + retval = SDL_MUTEX_TIMEDOUT; + } } - - res = sceKernelTryLockLwMutex(&mutex->lock, 1); - switch (res) { - case SCE_KERNEL_ERROR_OK: - return 0; - break; - case SCE_KERNEL_ERROR_WAIT_TIMEOUT: - return SDL_MUTEX_TIMEDOUT; - break; - default: - return SDL_SetError("Error trying to lock mutex: %lx", res); - break; - } - - return -1; -#endif /* SDL_THREADS_DISABLED */ +#endif // SDL_THREADS_DISABLED + return retval; } -/* 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 - SceInt32 res = 0; - - if (mutex == NULL) { - return 0; +#ifndef SDL_THREADS_DISABLED + if (mutex != NULL) { + const SceInt32 res = sceKernelUnlockLwMutex(&mutex->lock, 1); + SDL_assert(res == 0); // assume we're in a lot of trouble if this assert fails. } - - res = sceKernelUnlockLwMutex(&mutex->lock, 1); - if (res != 0) { - return SDL_SetError("Error trying to unlock mutex: %lx", res); - } - - return 0; -#endif /* SDL_THREADS_DISABLED */ +#endif // SDL_THREADS_DISABLED } -#endif /* SDL_THREAD_PSP */ +#endif // SDL_THREAD_PSP diff --git a/src/thread/pthread/SDL_sysmutex.c b/src/thread/pthread/SDL_sysmutex.c index 5e3dd696e1..4b921e1a48 100644 --- a/src/thread/pthread/SDL_sysmutex.c +++ b/src/thread/pthread/SDL_sysmutex.c @@ -30,7 +30,7 @@ SDL_Mutex *SDL_CreateMutex(void) SDL_Mutex *mutex; pthread_mutexattr_t attr; - /* Allocate the structure */ + // Allocate the structure mutex = (SDL_Mutex *)SDL_calloc(1, sizeof(*mutex)); if (mutex) { pthread_mutexattr_init(&attr); @@ -39,7 +39,7 @@ SDL_Mutex *SDL_CreateMutex(void) #elif defined(SDL_THREAD_PTHREAD_RECURSIVE_MUTEX_NP) pthread_mutexattr_setkind_np(&attr, PTHREAD_MUTEX_RECURSIVE_NP); #else - /* No extra attributes necessary */ + // No extra attributes necessary #endif if (pthread_mutex_init(&mutex->id, &attr) != 0) { SDL_SetError("pthread_mutex_init() failed"); @@ -60,116 +60,96 @@ 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 { + if (mutex != NULL) { #ifdef FAKE_RECURSIVE_MUTEX - pthread_t this_thread; -#endif - - if (mutex == NULL) { - return 0; - } - -#ifdef FAKE_RECURSIVE_MUTEX - this_thread = pthread_self(); - 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. - */ - if (pthread_mutex_lock(&mutex->id) == 0) { + pthread_t this_thread = pthread_self(); + 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. + */ + const int rc = pthread_mutex_lock(&mutex->id); + SDL_assert(rc == 0); // assume we're in a lot of trouble if this assert fails. mutex->owner = this_thread; mutex->recursive = 0; - } else { - return SDL_SetError("pthread_mutex_lock() failed"); } - } #else - if (pthread_mutex_lock(&mutex->id) != 0) { - return SDL_SetError("pthread_mutex_lock() failed"); - } + const int rc = pthread_mutex_lock(&mutex->id); + SDL_assert(rc == 0); // assume we're in a lot of trouble if this assert fails. #endif - return 0; + } } int SDL_TryLockMutex(SDL_Mutex *mutex) { - int retval; - int result; -#ifdef FAKE_RECURSIVE_MUTEX - pthread_t this_thread; -#endif + int retval = 0; - if (mutex == NULL) { - return 0; - } - - retval = 0; + if (mutex != NULL) { #ifdef FAKE_RECURSIVE_MUTEX - this_thread = pthread_self(); - 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. - */ - result = pthread_mutex_trylock(&mutex->id); - if (result == 0) { - mutex->owner = this_thread; - mutex->recursive = 0; - } else if (result == EBUSY) { - retval = SDL_MUTEX_TIMEDOUT; + pthread_t this_thread = pthread_self(); + if (mutex->owner == this_thread) { + ++mutex->recursive; } else { - retval = SDL_SetError("pthread_mutex_trylock() failed"); + /* The order of operations is important. + We set the locking thread id after we obtain the lock + so unlocks from other threads will fail. + */ + const int result = pthread_mutex_trylock(&mutex->id); + if (result == 0) { + mutex->owner = this_thread; + mutex->recursive = 0; + } else if (result == EBUSY) { + retval = SDL_MUTEX_TIMEDOUT; + } else { + SDL_assert(!"Error trying to lock mutex"); // assume we're in a lot of trouble if this assert fails. + retval = SDL_MUTEX_TIMEDOUT; + } } - } #else - result = pthread_mutex_trylock(&mutex->id); - if (result != 0) { - if (result == EBUSY) { - retval = SDL_MUTEX_TIMEDOUT; - } else { - retval = SDL_SetError("pthread_mutex_trylock() failed"); + const int result = pthread_mutex_trylock(&mutex->id); + if (result != 0) { + if (result == EBUSY) { + retval = SDL_MUTEX_TIMEDOUT; + } else { + SDL_assert(!"Error trying to lock mutex"); // assume we're in a lot of trouble if this assert fails. + retval = SDL_MUTEX_TIMEDOUT; + } } - } #endif + } + return retval; } -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 { - if (mutex == NULL) { - return 0; - } - + if (mutex != NULL) { #ifdef FAKE_RECURSIVE_MUTEX - /* We can only unlock the mutex if we own it */ - if (pthread_self() == mutex->owner) { - if (mutex->recursive) { - --mutex->recursive; + // We can only unlock the mutex if we own it + if (pthread_self() == mutex->owner) { + 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; + pthread_mutex_unlock(&mutex->id); + } } 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; - pthread_mutex_unlock(&mutex->id); + return SDL_SetError("mutex not owned by this thread"); } - } else { - return SDL_SetError("mutex not owned by this thread"); - } #else - if (pthread_mutex_unlock(&mutex->id) != 0) { - return SDL_SetError("pthread_mutex_unlock() failed"); + const int rc = pthread_mutex_unlock(&mutex->id); + SDL_assert(rc == 0); // assume we're in a lot of trouble if this assert fails. +#endif // FAKE_RECURSIVE_MUTEX } -#endif /* FAKE_RECURSIVE_MUTEX */ - - return 0; } diff --git a/src/thread/pthread/SDL_sysrwlock.c b/src/thread/pthread/SDL_sysrwlock.c index 3624d369e9..485264e85e 100644 --- a/src/thread/pthread/SDL_sysrwlock.c +++ b/src/thread/pthread/SDL_sysrwlock.c @@ -55,42 +55,36 @@ void SDL_DestroyRWLock(SDL_RWLock *rwlock) } } -int SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +void SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (rwlock == NULL) { - return SDL_InvalidParamError("rwlock"); - } else if (pthread_rwlock_rdlock(&rwlock->id) != 0) { - return SDL_SetError("pthread_rwlock_rdlock() failed"); + if (rwlock) { + const int rc = pthread_rwlock_rdlock(&rwlock->id); + SDL_assert(rc == 0); // assume we're in a lot of trouble if this assert fails. } - return 0; } -int SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +void SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (rwlock == NULL) { - return SDL_InvalidParamError("rwlock"); - } else if (pthread_rwlock_wrlock(&rwlock->id) != 0) { - return SDL_SetError("pthread_rwlock_wrlock() failed"); + if (rwlock) { + const int rc = pthread_rwlock_wrlock(&rwlock->id); + SDL_assert(rc == 0); // assume we're in a lot of trouble if this assert fails. } - return 0; } int SDL_TryLockRWLockForReading(SDL_RWLock *rwlock) { int retval = 0; - if (rwlock == NULL) { - retval = SDL_InvalidParamError("rwlock"); - } else { + if (rwlock) { const int result = pthread_rwlock_tryrdlock(&rwlock->id); if (result != 0) { - if (result == EBUSY) { - retval = SDL_RWLOCK_TIMEDOUT; - } else { - retval = SDL_SetError("pthread_rwlock_tryrdlock() failed"); + retval = SDL_RWLOCK_TIMEDOUT; + if (result != EBUSY) { + SDL_assert(!"Error trying to lock rwlock for reading"); // assume we're in a lot of trouble if this assert fails. } } } + return retval; } @@ -98,15 +92,12 @@ int SDL_TryLockRWLockForWriting(SDL_RWLock *rwlock) { int retval = 0; - if (rwlock == NULL) { - retval = SDL_InvalidParamError("rwlock"); - } else { + if (rwlock) { const int result = pthread_rwlock_trywrlock(&rwlock->id); if (result != 0) { - if (result == EBUSY) { - retval = SDL_RWLOCK_TIMEDOUT; - } else { - retval = SDL_SetError("pthread_rwlock_tryrdlock() failed"); + retval = SDL_RWLOCK_TIMEDOUT; + if (result != EBUSY) { + SDL_assert(!"Error trying to lock rwlock for writing"); // assume we're in a lot of trouble if this assert fails. } } } @@ -114,13 +105,11 @@ int SDL_TryLockRWLockForWriting(SDL_RWLock *rwlock) return retval; } -int SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +void SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (rwlock == NULL) { - return SDL_InvalidParamError("rwlock"); - } else if (pthread_rwlock_unlock(&rwlock->id) != 0) { - return SDL_SetError("pthread_rwlock_unlock() failed"); + if (rwlock) { + const int rc = pthread_rwlock_unlock(&rwlock->id); + SDL_assert(rc == 0); // assume we're in a lot of trouble if this assert fails. } - return 0; } diff --git a/src/thread/stdcpp/SDL_sysmutex.cpp b/src/thread/stdcpp/SDL_sysmutex.cpp index 01181f09f5..3619c41398 100644 --- a/src/thread/stdcpp/SDL_sysmutex.cpp +++ b/src/thread/stdcpp/SDL_sysmutex.cpp @@ -27,74 +27,51 @@ extern "C" { #include #include "SDL_sysmutex_c.h" -#include +#include -/* Create a mutex */ -extern "C" SDL_Mutex * -SDL_CreateMutex(void) +extern "C" SDL_Mutex * SDL_CreateMutex(void) { - /* Allocate and initialize the mutex */ + // Allocate and initialize the mutex try { SDL_Mutex *mutex = new SDL_Mutex; return mutex; } catch (std::system_error &ex) { SDL_SetError("unable to create a C++ mutex: code=%d; %s", ex.code(), ex.what()); - return NULL; } catch (std::bad_alloc &) { SDL_OutOfMemory(); - return NULL; } + return NULL; } -/* Free the mutex */ -extern "C" void -SDL_DestroyMutex(SDL_Mutex *mutex) +extern "C" void SDL_DestroyMutex(SDL_Mutex *mutex) { if (mutex != NULL) { delete mutex; } } -/* Lock the mutex */ -extern "C" int -SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +extern "C" void SDL_LockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (mutex == NULL) { - return 0; - } - - try { - mutex->cpp_mutex.lock(); - return 0; - } catch (std::system_error &ex) { - return SDL_SetError("unable to lock a C++ mutex: code=%d; %s", ex.code(), ex.what()); + if (mutex != NULL) { + try { + mutex->cpp_mutex.lock(); + } catch (std::system_error &ex) { + SDL_assert(!"Error trying to lock mutex"); // assume we're in a lot of trouble if this assert fails. + //return SDL_SetError("unable to lock a C++ mutex: code=%d; %s", ex.code(), ex.what()); + } } } -/* TryLock the mutex */ -int SDL_TryLockMutex(SDL_Mutex *mutex) +extern "C" int SDL_TryLockMutex(SDL_Mutex *mutex) { - int retval = 0; - - if (mutex == NULL) { - return 0; - } - - if (mutex->cpp_mutex.try_lock() == false) { - retval = SDL_MUTEX_TIMEDOUT; - } - return retval; + return ((mutex == NULL) || mutex->cpp_mutex.try_lock()) ? 0 : SDL_MUTEX_TIMEDOUT; } /* Unlock the mutex */ -extern "C" int -SDL_UnlockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +extern "C" void SDL_UnlockMutex(SDL_Mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (mutex == NULL) { - return 0; + if (mutex != NULL) { + mutex->cpp_mutex.unlock(); } - - mutex->cpp_mutex.unlock(); - return 0; } diff --git a/src/thread/stdcpp/SDL_sysrwlock.cpp b/src/thread/stdcpp/SDL_sysrwlock.cpp index 72673cab06..e85f879a9c 100644 --- a/src/thread/stdcpp/SDL_sysrwlock.cpp +++ b/src/thread/stdcpp/SDL_sysrwlock.cpp @@ -30,10 +30,8 @@ struct SDL_RWLock SDL_threadID write_owner; }; -/* Create a rwlock */ extern "C" SDL_RWLock *SDL_CreateRWLock(void) { - /* Allocate and initialize the rwlock */ try { SDL_RWLock *rwlock = new SDL_RWLock; return rwlock; @@ -46,7 +44,6 @@ extern "C" SDL_RWLock *SDL_CreateRWLock(void) } } -/* Free the rwlock */ extern "C" void SDL_DestroyRWLock(SDL_RWLock *rwlock) { if (rwlock != NULL) { @@ -54,77 +51,64 @@ extern "C" void SDL_DestroyRWLock(SDL_RWLock *rwlock) } } -/* Lock the rwlock */ -extern "C" int SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +extern "C" void SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (!rwlock) { - return SDL_InvalidParamError("rwlock"); - } - - try { - rwlock->cpp_mutex.lock_shared(); - return 0; - } catch (std::system_error &ex) { - return SDL_SetError("unable to lock a C++ rwlock: code=%d; %s", ex.code(), ex.what()); + if (rwlock) { + try { + rwlock->cpp_mutex.lock_shared(); + } catch (std::system_error &ex) { + SDL_assert(!"Error trying to lock rwlock for reading"); // assume we're in a lot of trouble if this assert fails. + //return SDL_SetError("unable to lock a C++ rwlock: code=%d; %s", ex.code(), ex.what()); + } } } -/* Lock the rwlock for writing */ -extern "C" int SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +extern "C" void SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (!rwlock) { - return SDL_InvalidParamError("rwlock"); - } - - try { - rwlock->cpp_mutex.lock(); - rwlock->write_owner = SDL_ThreadID(); - return 0; - } catch (std::system_error &ex) { - return SDL_SetError("unable to lock a C++ rwlock: code=%d; %s", ex.code(), ex.what()); + if (rwlock) { + try { + rwlock->cpp_mutex.lock(); + rwlock->write_owner = SDL_ThreadID(); + } catch (std::system_error &ex) { + SDL_assert(!"Error trying to lock rwlock for writing"); // assume we're in a lot of trouble if this assert fails. + //return SDL_SetError("unable to lock a C++ rwlock: code=%d; %s", ex.code(), ex.what()); + } } } -/* TryLock the rwlock for reading */ -int SDL_TryLockRWLockForReading(SDL_RWLock *rwlock) +extern "C" int SDL_TryLockRWLockForReading(SDL_RWLock *rwlock) { int retval = 0; - - if (!rwlock) { - retval = SDL_InvalidParamError("rwlock"); - } else if (rwlock->cpp_mutex.try_lock_shared() == false) { - retval = SDL_RWLOCK_TIMEDOUT; + if (rwlock) { + if (rwlock->cpp_mutex.try_lock_shared() == false) { + retval = SDL_RWLOCK_TIMEDOUT; + } } return retval; } -/* TryLock the rwlock for writing */ -int SDL_TryLockRWLockForWriting(SDL_RWLock *rwlock) +extern "C" int SDL_TryLockRWLockForWriting(SDL_RWLock *rwlock) { int retval = 0; - - if (!rwlock) { - retval = SDL_InvalidParamError("rwlock"); - } else if (rwlock->cpp_mutex.try_lock() == false) { - retval = SDL_RWLOCK_TIMEDOUT; - } else { - rwlock->write_owner = SDL_ThreadID(); + if (rwlock) { + if (rwlock->cpp_mutex.try_lock() == false) { + retval = SDL_RWLOCK_TIMEDOUT; + } else { + rwlock->write_owner = SDL_ThreadID(); + } } return retval; } -/* Unlock the rwlock */ -extern "C" int -SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +extern "C" void SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (!rwlock) { - return SDL_InvalidParamError("rwlock"); - } else if (rwlock->write_owner == SDL_ThreadID()) { - rwlock->write_owner = 0; - rwlock->cpp_mutex.unlock(); - } else { - rwlock->cpp_mutex.unlock_shared(); + if (rwlock) { + if (rwlock->write_owner == SDL_ThreadID()) { + rwlock->write_owner = 0; + rwlock->cpp_mutex.unlock(); + } else { + rwlock->cpp_mutex.unlock_shared(); + } } - return 0; } diff --git a/src/thread/vita/SDL_sysmutex.c b/src/thread/vita/SDL_sysmutex.c index 5e5ee47f9f..1bc27780e8 100644 --- a/src/thread/vita/SDL_sysmutex.c +++ b/src/thread/vita/SDL_sysmutex.c @@ -32,24 +32,20 @@ struct SDL_Mutex SceKernelLwMutexWork lock; }; -/* Create a mutex */ SDL_Mutex *SDL_CreateMutex(void) { - SDL_Mutex *mutex = NULL; - SceInt32 res = 0; - - /* Allocate mutex memory */ - mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex)); + SDL_Mutex *mutex = (SDL_Mutex *)SDL_malloc(sizeof(*mutex)); if (mutex != NULL) { - - res = sceKernelCreateLwMutex( - &mutex->lock, - "SDL mutex", - SCE_KERNEL_MUTEX_ATTR_RECURSIVE, - 0, - NULL); + const SceInt32 res = sceKernelCreateLwMutex( + &mutex->lock, + "SDL mutex", + SCE_KERNEL_MUTEX_ATTR_RECURSIVE, + 0, + NULL); if (res < 0) { + SDL_free(mutex); + mutex = NULL; SDL_SetError("Error trying to create mutex: %x", res); } } else { @@ -58,7 +54,6 @@ SDL_Mutex *SDL_CreateMutex(void) return mutex; } -/* Free the mutex */ void SDL_DestroyMutex(SDL_Mutex *mutex) { if (mutex != NULL) { @@ -67,28 +62,16 @@ 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 - SceInt32 res = 0; - - if (mutex == NULL) { - return 0; +#ifndef SDL_THREADS_DISABLED + if (mutex != NULL) { + const SceInt32 res = sceKernelLockLwMutex(&mutex->lock, 1, NULL); + SDL_assert(res == SCE_KERNEL_OK); // assume we're in a lot of trouble if this assert fails. } - - res = sceKernelLockLwMutex(&mutex->lock, 1, NULL); - if (res != SCE_KERNEL_OK) { - return SDL_SetError("Error trying to lock mutex: %x", res); - } - - return 0; -#endif /* SDL_THREADS_DISABLED */ +#endif // SDL_THREADS_DISABLED } -/* Try to lock the mutex */ int SDL_TryLockMutex(SDL_Mutex *mutex) { #ifdef SDL_THREADS_DISABLED @@ -102,40 +85,24 @@ int SDL_TryLockMutex(SDL_Mutex *mutex) res = sceKernelTryLockLwMutex(&mutex->lock, 1); switch (res) { - case SCE_KERNEL_OK: - return 0; - break; - case SCE_KERNEL_ERROR_MUTEX_FAILED_TO_OWN: - return SDL_MUTEX_TIMEDOUT; - break; - default: - return SDL_SetError("Error trying to lock mutex: %x", res); - break; + case SCE_KERNEL_OK: return 0; + case SCE_KERNEL_ERROR_MUTEX_FAILED_TO_OWN: return SDL_MUTEX_TIMEDOUT; + default: break; } - return -1; -#endif /* SDL_THREADS_DISABLED */ + SDL_assert(!"Error trying to lock mutex"); // assume we're in a lot of trouble if this assert fails. + return SDL_MUTEX_TIMEDOUT; +#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 - SceInt32 res = 0; - - if (mutex == NULL) { - return 0; +#ifndef SDL_THREADS_DISABLED + if (mutex != NULL) { + const SceInt32 res = sceKernelUnlockLwMutex(&mutex->lock, 1); + SDL_assert(res == SCE_KERNEL_OK); // assume we're in a lot of trouble if this assert fails. } - - res = sceKernelUnlockLwMutex(&mutex->lock, 1); - if (res != 0) { - return SDL_SetError("Error trying to unlock mutex: %x", res); - } - - return 0; -#endif /* SDL_THREADS_DISABLED */ +#endif // SDL_THREADS_DISABLED } -#endif /* SDL_THREAD_VITA */ +#endif // SDL_THREAD_VITA diff --git a/src/thread/windows/SDL_sysmutex.c b/src/thread/windows/SDL_sysmutex.c index 16cc81f93b..d70aa1be16 100644 --- a/src/thread/windows/SDL_sysmutex.c +++ b/src/thread/windows/SDL_sysmutex.c @@ -76,12 +76,11 @@ static void SDL_DestroyMutex_srw(SDL_Mutex *mutex) SDL_free(mutex); } -static int SDL_LockMutex_srw(SDL_Mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +static void SDL_LockMutex_srw(SDL_Mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { SDL_mutex_srw *mutex = (SDL_mutex_srw *)_mutex; - DWORD this_thread; + const DWORD this_thread = GetCurrentThreadId(); - this_thread = GetCurrentThreadId(); if (mutex->owner == this_thread) { ++mutex->count; } else { @@ -94,16 +93,14 @@ static int SDL_LockMutex_srw(SDL_Mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* mutex->owner = this_thread; mutex->count = 1; } - return 0; } static int SDL_TryLockMutex_srw(SDL_Mutex *_mutex) { SDL_mutex_srw *mutex = (SDL_mutex_srw *)_mutex; - DWORD this_thread; + const DWORD this_thread = GetCurrentThreadId(); int retval = 0; - this_thread = GetCurrentThreadId(); if (mutex->owner == this_thread) { ++mutex->count; } else { @@ -118,7 +115,7 @@ static int SDL_TryLockMutex_srw(SDL_Mutex *_mutex) return retval; } -static int SDL_UnlockMutex_srw(SDL_Mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +static void SDL_UnlockMutex_srw(SDL_Mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { SDL_mutex_srw *mutex = (SDL_mutex_srw *)_mutex; @@ -128,10 +125,8 @@ static int SDL_UnlockMutex_srw(SDL_Mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS pReleaseSRWLockExclusive(&mutex->srw); } } else { - return SDL_SetError("mutex not owned by this thread"); + SDL_assert(!"mutex not owned by this thread"); // undefined behavior...! } - - return 0; } static const SDL_mutex_impl_t SDL_mutex_impl_srw = { @@ -147,16 +142,12 @@ static const SDL_mutex_impl_t SDL_mutex_impl_srw = { * Fallback Mutex implementation using Critical Sections (before Win 7) */ -/* Create a mutex */ static SDL_Mutex *SDL_CreateMutex_cs(void) { - SDL_mutex_cs *mutex; - - /* Allocate mutex memory */ - mutex = (SDL_mutex_cs *)SDL_malloc(sizeof(*mutex)); + SDL_mutex_cs *mutex = (SDL_mutex_cs *)SDL_malloc(sizeof(*mutex)); if (mutex != NULL) { - /* Initialize */ - /* On SMP systems, a non-zero spin count generally helps performance */ + // Initialize + // On SMP systems, a non-zero spin count generally helps performance #ifdef __WINRT__ InitializeCriticalSectionEx(&mutex->cs, 2000, 0); #else @@ -168,43 +159,29 @@ static SDL_Mutex *SDL_CreateMutex_cs(void) return (SDL_Mutex *)mutex; } -/* Free the mutex */ static void SDL_DestroyMutex_cs(SDL_Mutex *mutex_) { SDL_mutex_cs *mutex = (SDL_mutex_cs *)mutex_; - DeleteCriticalSection(&mutex->cs); SDL_free(mutex); } -/* Lock the mutex */ -static int SDL_LockMutex_cs(SDL_Mutex *mutex_) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +static void SDL_LockMutex_cs(SDL_Mutex *mutex_) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { SDL_mutex_cs *mutex = (SDL_mutex_cs *)mutex_; - EnterCriticalSection(&mutex->cs); - return 0; } -/* TryLock the mutex */ static int SDL_TryLockMutex_cs(SDL_Mutex *mutex_) { SDL_mutex_cs *mutex = (SDL_mutex_cs *)mutex_; - int retval = 0; - - if (TryEnterCriticalSection(&mutex->cs) == 0) { - retval = SDL_MUTEX_TIMEDOUT; - } - return retval; + return (TryEnterCriticalSection(&mutex->cs) == 0) ? SDL_MUTEX_TIMEDOUT : 0; } -/* Unlock the mutex */ -static int SDL_UnlockMutex_cs(SDL_Mutex *mutex_) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +static void SDL_UnlockMutex_cs(SDL_Mutex *mutex_) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { SDL_mutex_cs *mutex = (SDL_mutex_cs *)mutex_; - LeaveCriticalSection(&mutex->cs); - return 0; } static const SDL_mutex_impl_t SDL_mutex_impl_cs = { @@ -223,22 +200,22 @@ static const SDL_mutex_impl_t SDL_mutex_impl_cs = { SDL_Mutex *SDL_CreateMutex(void) { if (SDL_mutex_impl_active.Create == NULL) { - /* Default to fallback implementation */ + // Default to fallback implementation const SDL_mutex_impl_t *impl = &SDL_mutex_impl_cs; if (!SDL_GetHintBoolean(SDL_HINT_WINDOWS_FORCE_MUTEX_CRITICAL_SECTIONS, SDL_FALSE)) { #ifdef __WINRT__ - /* Link statically on this platform */ + // Link statically on this platform impl = &SDL_mutex_impl_srw; #else - /* Try faster implementation for Windows 7 and newer */ + // Try faster implementation for Windows 7 and newer HMODULE kernel32 = GetModuleHandle(TEXT("kernel32.dll")); if (kernel32) { - /* Requires Vista: */ + // Requires Vista: pInitializeSRWLock = (pfnInitializeSRWLock)GetProcAddress(kernel32, "InitializeSRWLock"); pReleaseSRWLockExclusive = (pfnReleaseSRWLockExclusive)GetProcAddress(kernel32, "ReleaseSRWLockExclusive"); pAcquireSRWLockExclusive = (pfnAcquireSRWLockExclusive)GetProcAddress(kernel32, "AcquireSRWLockExclusive"); - /* Requires 7: */ + // Requires 7: pTryAcquireSRWLockExclusive = (pfnTryAcquireSRWLockExclusive)GetProcAddress(kernel32, "TryAcquireSRWLockExclusive"); if (pInitializeSRWLock && pReleaseSRWLockExclusive && pAcquireSRWLockExclusive && pTryAcquireSRWLockExclusive) { impl = &SDL_mutex_impl_srw; @@ -247,7 +224,7 @@ SDL_Mutex *SDL_CreateMutex(void) #endif } - /* Copy instead of using pointer to save one level of indirection */ + // Copy instead of using pointer to save one level of indirection SDL_copyp(&SDL_mutex_impl_active, impl); } return SDL_mutex_impl_active.Create(); @@ -260,31 +237,23 @@ void SDL_DestroyMutex(SDL_Mutex *mutex) } } -int SDL_LockMutex(SDL_Mutex *mutex) +void SDL_LockMutex(SDL_Mutex *mutex) { - if (mutex == NULL) { - return 0; + if (mutex != NULL) { + SDL_mutex_impl_active.Lock(mutex); } - - return SDL_mutex_impl_active.Lock(mutex); } int SDL_TryLockMutex(SDL_Mutex *mutex) { - if (mutex == NULL) { - return 0; - } - - return SDL_mutex_impl_active.TryLock(mutex); + return mutex ? SDL_mutex_impl_active.TryLock(mutex) : 0; } -int SDL_UnlockMutex(SDL_Mutex *mutex) +void SDL_UnlockMutex(SDL_Mutex *mutex) { - if (mutex == NULL) { - return 0; + if (mutex != NULL) { + SDL_mutex_impl_active.Unlock(mutex); } - - return SDL_mutex_impl_active.Unlock(mutex); } -#endif /* SDL_THREAD_WINDOWS */ +#endif // SDL_THREAD_WINDOWS diff --git a/src/thread/windows/SDL_sysmutex_c.h b/src/thread/windows/SDL_sysmutex_c.h index 5a04e143e4..d495ffe49b 100644 --- a/src/thread/windows/SDL_sysmutex_c.h +++ b/src/thread/windows/SDL_sysmutex_c.h @@ -23,9 +23,9 @@ #include "../../core/windows/SDL_windows.h" typedef SDL_Mutex *(*pfnSDL_CreateMutex)(void); -typedef int (*pfnSDL_LockMutex)(SDL_Mutex *); +typedef void (*pfnSDL_LockMutex)(SDL_Mutex *); typedef int (*pfnSDL_TryLockMutex)(SDL_Mutex *); -typedef int (*pfnSDL_UnlockMutex)(SDL_Mutex *); +typedef void (*pfnSDL_UnlockMutex)(SDL_Mutex *); typedef void (*pfnSDL_DestroyMutex)(SDL_Mutex *); typedef enum diff --git a/src/thread/windows/SDL_sysrwlock_srw.c b/src/thread/windows/SDL_sysrwlock_srw.c index bec3cb07df..c22eecae8e 100644 --- a/src/thread/windows/SDL_sysrwlock_srw.c +++ b/src/thread/windows/SDL_sysrwlock_srw.c @@ -57,11 +57,11 @@ static pfnTryAcquireSRWLockExclusive pTryAcquireSRWLockExclusive = NULL; typedef SDL_RWLock *(*pfnSDL_CreateRWLock)(void); typedef void (*pfnSDL_DestroyRWLock)(SDL_RWLock *); -typedef int (*pfnSDL_LockRWLockForReading)(SDL_RWLock *); -typedef int (*pfnSDL_LockRWLockForWriting)(SDL_RWLock *); +typedef void (*pfnSDL_LockRWLockForReading)(SDL_RWLock *); +typedef void (*pfnSDL_LockRWLockForWriting)(SDL_RWLock *); typedef int (*pfnSDL_TryLockRWLockForReading)(SDL_RWLock *); typedef int (*pfnSDL_TryLockRWLockForWriting)(SDL_RWLock *); -typedef int (*pfnSDL_UnlockRWLock)(SDL_RWLock *); +typedef void (*pfnSDL_UnlockRWLock)(SDL_RWLock *); typedef struct SDL_rwlock_impl_t { @@ -104,35 +104,28 @@ static void SDL_DestroyRWLock_srw(SDL_RWLock *_rwlock) } } -static int SDL_LockRWLockForReading_srw(SDL_RWLock *_rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +static void SDL_LockRWLockForReading_srw(SDL_RWLock *_rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { SDL_rwlock_srw *rwlock = (SDL_rwlock_srw *) _rwlock; - if (rwlock == NULL) { - return SDL_InvalidParamError("rwlock"); + if (rwlock != NULL) { + pAcquireSRWLockShared(&rwlock->srw); } - pAcquireSRWLockShared(&rwlock->srw); - return 0; } -static int SDL_LockRWLockForWriting_srw(SDL_RWLock *_rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +static void SDL_LockRWLockForWriting_srw(SDL_RWLock *_rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { SDL_rwlock_srw *rwlock = (SDL_rwlock_srw *) _rwlock; - if (rwlock == NULL) { - return SDL_InvalidParamError("rwlock"); + if (rwlock != NULL) { + pAcquireSRWLockExclusive(&rwlock->srw); + rwlock->write_owner = SDL_ThreadID(); } - pAcquireSRWLockExclusive(&rwlock->srw); - rwlock->write_owner = SDL_ThreadID(); - return 0; } static int SDL_TryLockRWLockForReading_srw(SDL_RWLock *_rwlock) { SDL_rwlock_srw *rwlock = (SDL_rwlock_srw *) _rwlock; int retval = 0; - - if (rwlock == NULL) { - retval = SDL_InvalidParamError("rwlock"); - } else { + if (rwlock != NULL) { retval = pTryAcquireSRWLockShared(&rwlock->srw) ? 0 : SDL_RWLOCK_TIMEDOUT; } return retval; @@ -142,27 +135,23 @@ static int SDL_TryLockRWLockForWriting_srw(SDL_RWLock *_rwlock) { SDL_rwlock_srw *rwlock = (SDL_rwlock_srw *) _rwlock; int retval = 0; - - if (rwlock == NULL) { - retval = SDL_InvalidParamError("rwlock"); - } else { + if (rwlock != NULL) { retval = pTryAcquireSRWLockExclusive(&rwlock->srw) ? 0 : SDL_RWLOCK_TIMEDOUT; } return retval; } -static int SDL_UnlockRWLock_srw(SDL_RWLock *_rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +static void SDL_UnlockRWLock_srw(SDL_RWLock *_rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { SDL_rwlock_srw *rwlock = (SDL_rwlock_srw *) _rwlock; - if (rwlock == NULL) { - return SDL_InvalidParamError("rwlock"); - } else if (rwlock->write_owner == SDL_ThreadID()) { - rwlock->write_owner = 0; - pReleaseSRWLockExclusive(&rwlock->srw); - } else { - pReleaseSRWLockShared(&rwlock->srw); + if (rwlock != NULL) { + if (rwlock->write_owner == SDL_ThreadID()) { + rwlock->write_owner = 0; + pReleaseSRWLockExclusive(&rwlock->srw); + } else { + pReleaseSRWLockShared(&rwlock->srw); + } } - return 0; } static const SDL_rwlock_impl_t SDL_rwlock_impl_srw = { @@ -234,47 +223,34 @@ void SDL_DestroyRWLock(SDL_RWLock *rwlock) } } -int SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +void SDL_LockRWLockForReading(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (rwlock == NULL) { - return 0; + if (rwlock != NULL) { + SDL_rwlock_impl_active.LockForReading(rwlock); } - - return SDL_rwlock_impl_active.LockForReading(rwlock); } -int SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +void SDL_LockRWLockForWriting(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (rwlock == NULL) { - return 0; + if (rwlock != NULL) { + SDL_rwlock_impl_active.LockForWriting(rwlock); } - - return SDL_rwlock_impl_active.LockForWriting(rwlock); } int SDL_TryLockRWLockForReading(SDL_RWLock *rwlock) { - if (rwlock == NULL) { - return 0; - } - - return SDL_rwlock_impl_active.TryLockForReading(rwlock); + return rwlock ? SDL_rwlock_impl_active.TryLockForReading(rwlock) : 0; } int SDL_TryLockRWLockForWriting(SDL_RWLock *rwlock) { - if (rwlock == NULL) { - return 0; - } - - return SDL_rwlock_impl_active.TryLockForWriting(rwlock); + return rwlock ? SDL_rwlock_impl_active.TryLockForWriting(rwlock) : 0; } -int SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */ +void SDL_UnlockRWLock(SDL_RWLock *rwlock) SDL_NO_THREAD_SAFETY_ANALYSIS // clang doesn't know about NULL mutexes { - if (rwlock == NULL) { - return 0; + if (rwlock != NULL) { + SDL_rwlock_impl_active.Unlock(rwlock); } - - return SDL_rwlock_impl_active.Unlock(rwlock); } + diff --git a/test/testlock.c b/test/testlock.c index e0413d87d9..edf259ad46 100644 --- a/test/testlock.c +++ b/test/testlock.c @@ -80,17 +80,12 @@ Run(void *data) SDL_Log("Thread %lu: starting up", SDL_ThreadID()); while (!SDL_AtomicGet(&doterminate)) { SDL_Log("Thread %lu: ready to work\n", SDL_ThreadID()); - if (SDL_LockMutex(mutex) < 0) { - SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Couldn't lock mutex: %s", SDL_GetError()); - exit(1); - } + SDL_LockMutex(mutex); SDL_Log("Thread %lu: start work!\n", SDL_ThreadID()); SDL_Delay(1 * worktime); SDL_Log("Thread %lu: work done!\n", SDL_ThreadID()); - if (SDL_UnlockMutex(mutex) < 0) { - SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Couldn't unlock mutex: %s", SDL_GetError()); - exit(1); - } + SDL_UnlockMutex(mutex); + /* If this sleep isn't done, then threads may starve */ SDL_Delay(10); } diff --git a/test/testrwlock.c b/test/testrwlock.c index cada79e559..7952a2ef74 100644 --- a/test/testrwlock.c +++ b/test/testrwlock.c @@ -33,22 +33,21 @@ static void DoWork(const int workticks) /* "Work" */ const SDL_threadID tid = SDL_ThreadID(); const SDL_bool is_reader = tid != mainthread; const char *typestr = is_reader ? "Reader" : "Writer"; - int rc; SDL_Log("%s Thread %lu: ready to work\n", typestr, (unsigned long) tid); - rc = is_reader ? SDL_LockRWLockForReading(rwlock) : SDL_LockRWLockForWriting(rwlock); - if (rc < 0) { - SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "%s Thread %lu: Couldn't lock rwlock: %s", typestr, (unsigned long) tid, SDL_GetError()); + if (is_reader) { + SDL_LockRWLockForReading(rwlock); } else { - SDL_Log("%s Thread %lu: start work!\n", typestr, (unsigned long) tid); - SDL_Delay(workticks); - SDL_Log("%s Thread %lu: work done!\n", typestr, (unsigned long) tid); - if (SDL_UnlockRWLock(rwlock) < 0) { - SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "%s Thread %lu: Couldn't unlock rwlock: %s", typestr, (unsigned long) tid, SDL_GetError()); - } - /* If this sleep isn't done, then threads may starve */ - SDL_Delay(10); + SDL_LockRWLockForWriting(rwlock); } + + SDL_Log("%s Thread %lu: start work!\n", typestr, (unsigned long) tid); + SDL_Delay(workticks); + SDL_Log("%s Thread %lu: work done!\n", typestr, (unsigned long) tid); + SDL_UnlockRWLock(rwlock); + + /* If this sleep isn't done, then threads may starve */ + SDL_Delay(10); } static int SDLCALL