[Mesa-dev] [PATCH 01/13] c11/threads: Assert that mtx is non-NULL and check return values.

[Timothy Arceri]

On Thu, 2015-08-06 at 17:10 -0700, Matt Turner wrote:
> Passing NULL to C11 threads functions isn't safe, so there's no need for
> our implementation to handle it. Cuts about 1k of .text.
> 
>    text     data      bss      dec      hex  filename
> 5009514   198440    26328  5234282   4fde6a  i965_dri.so before
> 5008346   198440    26328  5233114   4fd9da  i965_dri.so after
> ---
>  include/c11/threads_posix.h | 48 ++++++++++++++++++++++--------------------
> ---
>  1 file changed, 23 insertions(+), 25 deletions(-)
> 
> diff --git a/include/c11/threads_posix.h b/include/c11/threads_posix.h
> index 2182c28..3def6c4 100644
> --- a/include/c11/threads_posix.h
> +++ b/include/c11/threads_posix.h
> @@ -102,9 +102,8 @@ call_once(once_flag *flag, void (*func)(void))
>  static inline int
>  cnd_broadcast(cnd_t *cond)
>  {
> -    if (!cond) return thrd_error;
> -    pthread_cond_broadcast(cond);
> -    return thrd_success;
> +    assert(cond != NULL);
> +    return (pthread_cond_broadcast(cond) == 0) ? thrd_success : thrd_error;
>  }
>  
>  // 7.25.3.2
> @@ -119,18 +118,16 @@ cnd_destroy(cnd_t *cond)
>  static inline int
>  cnd_init(cnd_t *cond)
>  {
> -    if (!cond) return thrd_error;
> -    pthread_cond_init(cond, NULL);
> -    return thrd_success;
> +    assert(cond != NULL);
> +    return (pthread_cond_init(cond, NULL) == 0) ? thrd_success : 
> thrd_error;
>  }
>  
>  // 7.25.3.4
>  static inline int
>  cnd_signal(cnd_t *cond)
>  {
> -    if (!cond) return thrd_error;
> -    pthread_cond_signal(cond);
> -    return thrd_success;
> +    assert(cond != NULL);
> +    return (pthread_cond_signal(cond) == 0) ? thrd_success : thrd_error;
>  }
>  
>  // 7.25.3.5
> @@ -139,7 +136,8 @@ cnd_timedwait(cnd_t *cond, mtx_t *mtx, const xtime *xt)
>  {
>      struct timespec abs_time;
>      int rt;
> -    if (!cond || !mtx || !xt) return thrd_error;
> +    assert(mtx != NULL);
> +    assert(cond != NULL);

Even though its not used in this wrapper I guess you could have assert(xt !=
NULL); to catch any potential misuse of the real c11/threads here. Up to you
if you think its important.

Reviewed-by: Timothy Arceri <t_arceri at yahoo.com.au>


>      rt = pthread_cond_timedwait(cond, mtx, &abs_time);
>      if (rt == ETIMEDOUT)
>          return thrd_busy;
> @@ -150,9 +148,9 @@ cnd_timedwait(cnd_t *cond, mtx_t *mtx, const xtime *xt)
>  static inline int
>  cnd_wait(cnd_t *cond, mtx_t *mtx)
>  {
> -    if (!cond || !mtx) return thrd_error;
> -    pthread_cond_wait(cond, mtx);
> -    return thrd_success;
> +    assert(mtx != NULL);
> +    assert(cond != NULL);
> +    return (pthread_cond_wait(cond, mtx) == 0) ? thrd_success : thrd_error;
>  }
>  
>  
> @@ -161,7 +159,7 @@ cnd_wait(cnd_t *cond, mtx_t *mtx)
>  static inline void
>  mtx_destroy(mtx_t *mtx)
>  {
> -    assert(mtx);
> +    assert(mtx != NULL);
>      pthread_mutex_destroy(mtx);
>  }
>  
> @@ -170,7 +168,7 @@ static inline int
>  mtx_init(mtx_t *mtx, int type)
>  {
>      pthread_mutexattr_t attr;
> -    if (!mtx) return thrd_error;
> +    assert(mtx != NULL);
>      if (type != mtx_plain && type != mtx_timed && type != mtx_try
>        && type != (mtx_plain|mtx_recursive)
>        && type != (mtx_timed|mtx_recursive)
> @@ -188,9 +186,8 @@ mtx_init(mtx_t *mtx, int type)
>  static inline int
>  mtx_lock(mtx_t *mtx)
>  {
> -    if (!mtx) return thrd_error;
> -    pthread_mutex_lock(mtx);
> -    return thrd_success;
> +    assert(mtx != NULL);
> +    return (pthread_mutex_lock(mtx) == 0) ? thrd_success : thrd_error;
>  }
>  
>  static inline int
> @@ -203,7 +200,9 @@ thrd_yield(void);
>  static inline int
>  mtx_timedlock(mtx_t *mtx, const xtime *xt)
>  {
> -    if (!mtx || !xt) return thrd_error;
> +    assert(mtx != NULL);
> +    assert(xt != NULL);
> +
>      {
>  #ifdef EMULATED_THREADS_USE_NATIVE_TIMEDLOCK
>      struct timespec ts;
> @@ -233,7 +232,7 @@ mtx_timedlock(mtx_t *mtx, const xtime *xt)
>  static inline int
>  mtx_trylock(mtx_t *mtx)
>  {
> -    if (!mtx) return thrd_error;
> +    assert(mtx != NULL);
>      return (pthread_mutex_trylock(mtx) == 0) ? thrd_success : thrd_busy;
>  }
>  
> @@ -241,9 +240,8 @@ mtx_trylock(mtx_t *mtx)
>  static inline int
>  mtx_unlock(mtx_t *mtx)
>  {
> -    if (!mtx) return thrd_error;
> -    pthread_mutex_unlock(mtx);
> -    return thrd_success;
> +    assert(mtx != NULL);
> +    return (pthread_mutex_unlock(mtx) == 0) ? thrd_success : thrd_error;
>  }
>  
>  
> @@ -253,7 +251,7 @@ static inline int
>  thrd_create(thrd_t *thr, thrd_start_t func, void *arg)
>  {
>      struct impl_thrd_param *pack;
> -    if (!thr) return thrd_error;
> +    assert(thr != NULL);
>      pack = (struct impl_thrd_param *)malloc(sizeof(struct 
> impl_thrd_param));
>      if (!pack) return thrd_nomem;
>      pack->func = func;
> @@ -329,7 +327,7 @@ thrd_yield(void)
>  static inline int
>  tss_create(tss_t *key, tss_dtor_t dtor)
>  {
> -    if (!key) return thrd_error;
> +    assert(key != NULL);
>      return (pthread_key_create(key, dtor) == 0) ? thrd_success : 
> thrd_error;
>  }
>