From 39b644a0e270df453c53d6060cd364391bb1c512 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 4 Mar 2014 10:55:39 -0800 Subject: [PATCH] Remove dead NULL checks from pthread code. GCC is removing these checks anyway because it knows the arguments must be non-null, so leaving this code around is just confusing. We know from experience that people were shipping code with locking bugs because they weren't checking for error returns. Failing hard like glibc does seems the better choice. (And it's what the checked in code was already doing; this patch doesn't change that. It just makes it more obvious that that's what's going on.) Change-Id: I167c6d7c0a296822baf0cb9b43b97821eba7ab35 --- libc/bionic/pthread_cond.cpp | 15 --------- libc/bionic/pthread_mutex.cpp | 51 ++++++++++-------------------- libc/bionic/pthread_rwlock.cpp | 35 -------------------- libc/bionic/pthread_setname_np.cpp | 4 --- 4 files changed, 17 insertions(+), 88 deletions(-) diff --git a/libc/bionic/pthread_cond.cpp b/libc/bionic/pthread_cond.cpp index e570602be..213bcd733 100644 --- a/libc/bionic/pthread_cond.cpp +++ b/libc/bionic/pthread_cond.cpp @@ -96,9 +96,6 @@ int pthread_condattr_setclock(pthread_condattr_t* attr, clockid_t clock) { } int pthread_condattr_destroy(pthread_condattr_t* attr) { - if (attr == NULL) { - return EINVAL; - } *attr = 0xdeada11d; return 0; } @@ -112,10 +109,6 @@ int pthread_condattr_destroy(pthread_condattr_t* attr) { // XXX then the signal will be lost. int pthread_cond_init(pthread_cond_t* cond, const pthread_condattr_t* attr) { - if (cond == NULL) { - return EINVAL; - } - if (attr != NULL) { cond->value = (*attr & COND_FLAGS_MASK); } else { @@ -126,10 +119,6 @@ int pthread_cond_init(pthread_cond_t* cond, const pthread_condattr_t* attr) { } int pthread_cond_destroy(pthread_cond_t* cond) { - if (cond == NULL) { - return EINVAL; - } - cond->value = 0xdeadc04d; return 0; } @@ -138,10 +127,6 @@ int pthread_cond_destroy(pthread_cond_t* cond) { // pthread_cond_signal to atomically decrement the counter // then wake up 'counter' threads. static int __pthread_cond_pulse(pthread_cond_t* cond, int counter) { - if (__predict_false(cond == NULL)) { - return EINVAL; - } - int flags = (cond->value & COND_FLAGS_MASK); while (true) { int old_value = cond->value; diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp index 90726052e..a2e7b25dc 100644 --- a/libc/bionic/pthread_mutex.cpp +++ b/libc/bionic/pthread_mutex.cpp @@ -207,55 +207,42 @@ extern void pthread_debug_mutex_unlock_check(pthread_mutex_t *mutex); int pthread_mutexattr_init(pthread_mutexattr_t *attr) { - if (attr) { - *attr = PTHREAD_MUTEX_DEFAULT; - return 0; - } else { - return EINVAL; - } + *attr = PTHREAD_MUTEX_DEFAULT; + return 0; } int pthread_mutexattr_destroy(pthread_mutexattr_t *attr) { - if (attr) { - *attr = -1; - return 0; - } else { - return EINVAL; - } + *attr = -1; + return 0; } -int pthread_mutexattr_gettype(const pthread_mutexattr_t *attr, int *type) +int pthread_mutexattr_gettype(const pthread_mutexattr_t *attr, int *type_p) { - if (attr) { - int atype = (*attr & MUTEXATTR_TYPE_MASK); + int type = (*attr & MUTEXATTR_TYPE_MASK); - if (atype >= PTHREAD_MUTEX_NORMAL && - atype <= PTHREAD_MUTEX_ERRORCHECK) { - *type = atype; - return 0; - } + if (type < PTHREAD_MUTEX_NORMAL || type > PTHREAD_MUTEX_ERRORCHECK) { + return EINVAL; } - return EINVAL; + + *type_p = type; + return 0; } int pthread_mutexattr_settype(pthread_mutexattr_t *attr, int type) { - if (attr && type >= PTHREAD_MUTEX_NORMAL && - type <= PTHREAD_MUTEX_ERRORCHECK ) { - *attr = (*attr & ~MUTEXATTR_TYPE_MASK) | type; - return 0; + if (type < PTHREAD_MUTEX_NORMAL || type > PTHREAD_MUTEX_ERRORCHECK ) { + return EINVAL; } - return EINVAL; + + *attr = (*attr & ~MUTEXATTR_TYPE_MASK) | type; + return 0; } /* process-shared mutexes are not supported at the moment */ int pthread_mutexattr_setpshared(pthread_mutexattr_t *attr, int pshared) { - if (!attr) - return EINVAL; - switch (pshared) { case PTHREAD_PROCESS_PRIVATE: *attr &= ~MUTEXATTR_SHARED_MASK; @@ -274,11 +261,7 @@ int pthread_mutexattr_setpshared(pthread_mutexattr_t *attr, int pshared) } int pthread_mutexattr_getpshared(const pthread_mutexattr_t* attr, int* pshared) { - if (!attr || !pshared) - return EINVAL; - - *pshared = (*attr & MUTEXATTR_SHARED_MASK) ? PTHREAD_PROCESS_SHARED - : PTHREAD_PROCESS_PRIVATE; + *pshared = (*attr & MUTEXATTR_SHARED_MASK) ? PTHREAD_PROCESS_SHARED : PTHREAD_PROCESS_PRIVATE; return 0; } diff --git a/libc/bionic/pthread_rwlock.cpp b/libc/bionic/pthread_rwlock.cpp index 0182ef307..dfb431578 100644 --- a/libc/bionic/pthread_rwlock.cpp +++ b/libc/bionic/pthread_rwlock.cpp @@ -60,27 +60,18 @@ extern pthread_internal_t* __get_thread(void); int pthread_rwlockattr_init(pthread_rwlockattr_t *attr) { - if (!attr) - return EINVAL; - *attr = PTHREAD_PROCESS_PRIVATE; return 0; } int pthread_rwlockattr_destroy(pthread_rwlockattr_t *attr) { - if (!attr) - return EINVAL; - *attr = -1; return 0; } int pthread_rwlockattr_setpshared(pthread_rwlockattr_t *attr, int pshared) { - if (!attr) - return EINVAL; - switch (pshared) { case PTHREAD_PROCESS_PRIVATE: case PTHREAD_PROCESS_SHARED: @@ -92,9 +83,6 @@ int pthread_rwlockattr_setpshared(pthread_rwlockattr_t *attr, int pshared) } int pthread_rwlockattr_getpshared(const pthread_rwlockattr_t* attr, int* pshared) { - if (!attr || !pshared) - return EINVAL; - *pshared = *attr; return 0; } @@ -107,9 +95,6 @@ int pthread_rwlock_init(pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *at pthread_condattr_t cond_attr0; int ret; - if (rwlock == NULL) - return EINVAL; - if (attr && *attr == PTHREAD_PROCESS_SHARED) { lock_attr = &lock_attr0; pthread_mutexattr_init(lock_attr); @@ -140,9 +125,6 @@ int pthread_rwlock_init(pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *at int pthread_rwlock_destroy(pthread_rwlock_t *rwlock) { - if (rwlock == NULL) - return EINVAL; - if (rwlock->numLocks > 0) return EBUSY; @@ -197,10 +179,6 @@ static void _pthread_rwlock_pulse(pthread_rwlock_t *rwlock) static int __pthread_rwlock_timedrdlock(pthread_rwlock_t* rwlock, const timespec* abs_timeout) { int ret = 0; - if (rwlock == NULL) { - return EINVAL; - } - pthread_mutex_lock(&rwlock->lock); int tid = __get_thread()->tid; if (__predict_false(!read_precondition(rwlock, tid))) { @@ -222,10 +200,6 @@ EXIT: static int __pthread_rwlock_timedwrlock(pthread_rwlock_t* rwlock, const timespec* abs_timeout) { int ret = 0; - if (rwlock == NULL) { - return EINVAL; - } - pthread_mutex_lock(&rwlock->lock); int tid = __get_thread()->tid; if (__predict_false(!write_precondition(rwlock, tid))) { @@ -256,9 +230,6 @@ int pthread_rwlock_tryrdlock(pthread_rwlock_t *rwlock) { int ret = 0; - if (rwlock == NULL) - return EINVAL; - pthread_mutex_lock(&rwlock->lock); if (__predict_false(!read_precondition(rwlock, __get_thread()->tid))) ret = EBUSY; @@ -281,9 +252,6 @@ int pthread_rwlock_trywrlock(pthread_rwlock_t *rwlock) { int ret = 0; - if (rwlock == NULL) - return EINVAL; - pthread_mutex_lock(&rwlock->lock); int tid = __get_thread()->tid; if (__predict_false(!write_precondition(rwlock, tid))) { @@ -304,9 +272,6 @@ int pthread_rwlock_unlock(pthread_rwlock_t *rwlock) { int ret = 0; - if (rwlock == NULL) - return EINVAL; - pthread_mutex_lock(&rwlock->lock); /* The lock must be held */ diff --git a/libc/bionic/pthread_setname_np.cpp b/libc/bionic/pthread_setname_np.cpp index e0ecace0b..1ddf81044 100644 --- a/libc/bionic/pthread_setname_np.cpp +++ b/libc/bionic/pthread_setname_np.cpp @@ -46,10 +46,6 @@ int pthread_setname_np(pthread_t t, const char* thread_name) { ErrnoRestorer errno_restorer; - if (thread_name == NULL) { - return EINVAL; - } - size_t thread_name_len = strlen(thread_name); if (thread_name_len >= MAX_TASK_COMM_LEN) { return ERANGE;