Skip to content

Commit

Permalink
Clean up return type checking for callers of H5TS routines.
Browse files Browse the repository at this point in the history
Signed-off-by: Quincey Koziol <[email protected]>
  • Loading branch information
qkoziol committed Mar 22, 2024
1 parent 0dea612 commit d78bc6b
Show file tree
Hide file tree
Showing 18 changed files with 145 additions and 193 deletions.
1 change: 0 additions & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,6 @@ set (H5TS_SOURCES
${HDF5_SRC_DIR}/H5TSpool.c
${HDF5_SRC_DIR}/H5TSpthread.c
${HDF5_SRC_DIR}/H5TSrwlock.c
${HDF5_SRC_DIR}/H5TStest.c
${HDF5_SRC_DIR}/H5TSthread.c
${HDF5_SRC_DIR}/H5TSwin.c
)
Expand Down
5 changes: 1 addition & 4 deletions src/H5CX.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@

#ifdef H5_HAVE_THREADSAFE
/*
* The per-thread API context. pthread_once() initializes a special
* key that will be used by all threads to create a stack specific to
* each thread individually. The association of contexts to threads will
* be handled by the pthread library.
* The per-thread API context.
*
* In order for this macro to work, H5CX_get_my_context() must be preceded
* by "H5CX_node_t *ctx =".
Expand Down
6 changes: 5 additions & 1 deletion src/H5Eint.c
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,15 @@ H5E__walk1_cb(int n, H5E_error1_t *err_desc, void *client_data)
MPI_Comm_rank(MPI_COMM_WORLD, &mpi_rank);
fprintf(stream, "MPI-process %d", mpi_rank);
} /* end if */
#ifdef H5_HAVE_THREADSAFE
else
fprintf(stream, "thread 0");
fprintf(stream, "thread %" PRIu64, H5TS_thread_id());
#endif
} /* end block */
#else
#ifdef H5_HAVE_THREADSAFE
fprintf(stream, "thread %" PRIu64, H5TS_thread_id());
#endif
#endif
fprintf(stream, ":\n");
} /* end if */
Expand Down
5 changes: 1 addition & 4 deletions src/H5Epkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,7 @@

#ifdef H5_HAVE_THREADSAFE
/*
* The per-thread error stack. pthread_once() initializes a special
* key that will be used by all threads to create a stack specific to
* each thread individually. The association of stacks to threads will
* be handled by the pthread library.
* The per-thread error stack.
*
* In order for this macro to work, H5E__get_my_stack() must be preceded
* by "H5E_t *estack =".
Expand Down
18 changes: 13 additions & 5 deletions src/H5TS.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
/*********************/

/* API threadsafety info */
H5TS_api_info_t H5TS_api_info_p;
H5TS_api_info_t H5TS_api_info_p = {H5TS_EX_LOCK_INIT, H5TS_MUTEX_INITIALIZER, 0};

/*****************************/
/* Library Private Variables */
Expand All @@ -77,11 +77,19 @@ H5TS_api_info_t H5TS_api_info_p;
*--------------------------------------------------------------------------
*/
herr_t
H5TSmutex_acquire(unsigned lock_count, bool *acquired){
H5TSmutex_acquire(unsigned lock_count, bool *acquired)
{
herr_t ret_value = SUCCEED;

FUNC_ENTER_API_NAMECHECK_ONLY

FUNC_LEAVE_API_NAMECHECK_ONLY(H5TS__mutex_acquire(lock_count, acquired))}
/* end H5TSmutex_acquire() */
/* Acquire the "attempt" lock */
if (H5_UNLIKELY(H5TS__mutex_acquire(lock_count, acquired) < 0))
HGOTO_DONE(FAIL);

done:
FUNC_LEAVE_API_NAMECHECK_ONLY(ret_value)
} /* end H5TSmutex_acquire() */

/*--------------------------------------------------------------------------
* Function: H5TSmutex_get_attempt_count
Expand Down Expand Up @@ -111,7 +119,7 @@ herr_t H5TSmutex_get_attempt_count(unsigned *count)

done:
/* Release the lock */
if (have_mutex)
if (H5_LIKELY(have_mutex))
if (H5_UNLIKELY(H5TS_mutex_unlock(&H5TS_api_info_p.attempt_mutex) < 0))
ret_value = FAIL;

Expand Down
24 changes: 12 additions & 12 deletions src/H5TSbarrier.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ H5TS__barrier_init(H5TS_barrier_t *barrier, uint64_t count)

/* Initialize the barrier */
#ifdef H5_HAVE_PTHREAD_BARRIER
if (pthread_barrier_init(barrier, NULL, count))
if (H5_UNLIKELY(pthread_barrier_init(barrier, NULL, count)))
HGOTO_DONE(FAIL);
#else
memcpy(barrier, &H5TS_barrier_def, sizeof(H5TS_barrier_def));
Expand Down Expand Up @@ -136,21 +136,21 @@ H5TS__barrier_wait(H5TS_barrier_t *barrier)
HGOTO_DONE(FAIL);

#ifdef H5_HAVE_PTHREAD_BARRIER
if (0 != pthread_barrier_wait(barrier))
if (H5_UNLIKELY(pthread_barrier_wait(barrier)))
HGOTO_DONE(FAIL);
#else
/* Acquire the mutex for the barrier */
if (H5_UNLIKELY(H5TS_mutex_lock(&barrier->mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&barrier->mutex) < 0))
HGOTO_DONE(FAIL);
have_mutex = true;

barrier->entered++;
if (barrier->entered < barrier->threshold) {
if (H5_UNLIKELY(H5TS_cond_wait(&barrier->cv, &barrier->mutex)))
if (H5_UNLIKELY(H5TS_cond_wait(&barrier->cv, &barrier->mutex) < 0))
HGOTO_DONE(FAIL);
}
else {
if (H5_UNLIKELY(H5TS_cond_broadcast(&barrier->cv)))
if (H5_UNLIKELY(H5TS_cond_broadcast(&barrier->cv) < 0))
HGOTO_DONE(FAIL);

/* Increment threshold count of threads for next barrier */
Expand All @@ -160,8 +160,8 @@ H5TS__barrier_wait(H5TS_barrier_t *barrier)

done:
#ifndef H5_HAVE_PTHREAD_BARRIER
if (have_mutex)
if (H5_UNLIKELY(H5TS_mutex_unlock(&barrier->mutex)))
if (H5_LIKELY(have_mutex))
if (H5_UNLIKELY(H5TS_mutex_unlock(&barrier->mutex) < 0))
ret_value = FAIL;
#endif

Expand Down Expand Up @@ -189,12 +189,12 @@ H5TS__barrier_destroy(H5TS_barrier_t *barrier)
HGOTO_DONE(FAIL);

#ifdef H5_HAVE_PTHREAD_BARRIER
if (0 != pthread_barrier_destroy(barrier))
if (H5_UNLIKELY(pthread_barrier_destroy(barrier)))
HGOTO_DONE(FAIL);
#else

/* Acquire the mutex for the barrier */
if (H5_UNLIKELY(H5TS_mutex_lock(&barrier->mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&barrier->mutex) < 0))
HGOTO_DONE(FAIL);

/* Check for barrier still in use */
Expand All @@ -204,16 +204,16 @@ H5TS__barrier_destroy(H5TS_barrier_t *barrier)
}

/* Release the barrier's mutex */
if (H5_UNLIKELY(H5TS_mutex_unlock(&barrier->mutex)))
if (H5_UNLIKELY(H5TS_mutex_unlock(&barrier->mutex) < 0))
HGOTO_DONE(FAIL);

/* Call the appropriate pthread destroy routines. We are committed
* to the destroy at this point, so call them all, even if one fails
* along the way.
*/
if (H5_UNLIKELY(H5TS_mutex_destroy(&barrier->mutex)))
if (H5_UNLIKELY(H5TS_mutex_destroy(&barrier->mutex) < 0))
ret_value = FAIL;
if (H5_UNLIKELY(H5TS_cond_destroy(&barrier->cv)))
if (H5_UNLIKELY(H5TS_cond_destroy(&barrier->cv) < 0))
ret_value = FAIL;
#endif

Expand Down
63 changes: 22 additions & 41 deletions src/H5TSexlock.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,6 @@
/* Local Macros */
/****************/

/* Excl lock initialization macro */
#ifdef H5_HAVE_PTHREAD_H
#define H5TS_EX_LOCK_INIT \
{ \
H5TS_MUTEX_INITIALIZER, /* mutex */ \
H5TS_COND_INITIALIZER, /* cond_var */ \
0, /* owner_thread */ \
0, /* lock_count */ \
false, /* disable_cancel */ \
0 /* previous_state */ \
}
#else
#define H5TS_EXL_LOCK_INIT \
{ \
H5TS_MUTEX_INITIALIZER, /* mutex */ \
H5TS_COND_INITIALIZER, /* cond_var */ \
0, /* owner_thread */ \
0 /* lock_count */ \
}
#endif

/******************/
/* Local Typedefs */
Expand Down Expand Up @@ -137,7 +117,7 @@ H5TS__ex_lock(H5TS_ex_lock_t *lock)
FUNC_ENTER_NOAPI_NAMECHECK_ONLY

/* Acquire the mutex for the lock */
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex) < 0))
HGOTO_DONE(FAIL);
have_mutex = true;

Expand All @@ -148,7 +128,8 @@ H5TS__ex_lock(H5TS_ex_lock_t *lock)
else {
/* Wait until the mutex is released by current owner thread */
while (lock->lock_count)
H5TS_cond_wait(&lock->cond_var, &lock->mutex);
if (H5_UNLIKELY(H5TS_cond_wait(&lock->cond_var, &lock->mutex) < 0))
HGOTO_DONE(FAIL);

/* After we've received the signal, take ownership of the lock */
lock->owner_thread = my_thread_id;
Expand All @@ -164,8 +145,8 @@ H5TS__ex_lock(H5TS_ex_lock_t *lock)
}

done:
if (have_mutex)
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex)))
if (H5_LIKELY(have_mutex))
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex) < 0))
ret_value = FAIL;

FUNC_LEAVE_NOAPI_NAMECHECK_ONLY(ret_value)
Expand All @@ -192,7 +173,7 @@ H5TS__ex_acquire(H5TS_ex_lock_t *lock, unsigned lock_count, bool *acquired)
FUNC_ENTER_PACKAGE_NAMECHECK_ONLY

/* Attempt to acquire the lock's mutex */
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex) < 0))
HGOTO_DONE(FAIL);
have_mutex = true;

Expand All @@ -216,8 +197,8 @@ H5TS__ex_acquire(H5TS_ex_lock_t *lock, unsigned lock_count, bool *acquired)

done:
/* Release the mutex, if acquired */
if (have_mutex)
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex)))
if (H5_LIKELY(have_mutex))
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex) < 0))
ret_value = FAIL;

FUNC_LEAVE_NOAPI_NAMECHECK_ONLY(ret_value)
Expand All @@ -242,7 +223,7 @@ H5TS__ex_release(H5TS_ex_lock_t *lock, unsigned int *lock_count)
FUNC_ENTER_NOAPI_NAMECHECK_ONLY

/* Attempt to acquire the lock's mutex */
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex) < 0))
HGOTO_DONE(FAIL);
have_mutex = true;

Expand All @@ -251,12 +232,12 @@ H5TS__ex_release(H5TS_ex_lock_t *lock, unsigned int *lock_count)
lock->lock_count = 0;

/* Signal the condition variable, to wake any thread waiting on the lock */
if (H5_UNLIKELY(H5TS_cond_signal(&lock->cond_var)))
if (H5_UNLIKELY(H5TS_cond_signal(&lock->cond_var) < 0))
HGOTO_DONE(FAIL);

done:
if (have_mutex)
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex)))
if (H5_LIKELY(have_mutex))
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex) < 0))
ret_value = FAIL;

FUNC_LEAVE_NOAPI_NAMECHECK_ONLY(ret_value)
Expand All @@ -281,7 +262,7 @@ H5TS__ex_unlock(H5TS_ex_lock_t *lock)
FUNC_ENTER_NOAPI_NAMECHECK_ONLY

/* Acquire the mutex for the lock */
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex) < 0))
HGOTO_DONE(FAIL);
have_mutex = true;

Expand All @@ -299,13 +280,13 @@ H5TS__ex_unlock(H5TS_ex_lock_t *lock)
/* If the lock count drops to zero, signal the condition variable, to
* wake any thread waiting on the lock.
*/
if (H5_UNLIKELY(H5TS_cond_signal(&lock->cond_var)))
if (H5_UNLIKELY(H5TS_cond_signal(&lock->cond_var) < 0))
HGOTO_DONE(FAIL);
}

done:
if (have_mutex)
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex)))
if (H5_LIKELY(have_mutex))
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex) < 0))
ret_value = FAIL;

FUNC_LEAVE_NOAPI_NAMECHECK_ONLY(ret_value)
Expand Down Expand Up @@ -334,7 +315,7 @@ H5TS__ex_lock_destroy(H5TS_ex_lock_t *lock)
HGOTO_DONE(FAIL);

/* Acquire the mutex for the lock */
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&lock->mutex) < 0))
HGOTO_DONE(FAIL);
have_mutex = true;

Expand All @@ -343,22 +324,22 @@ H5TS__ex_lock_destroy(H5TS_ex_lock_t *lock)
HGOTO_DONE(FAIL);

/* Release the mutex for the lock */
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex)))
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex) < 0))
HGOTO_DONE(FAIL);
have_mutex = false;

/* Call the appropriate destroy routines. We are committed
* to the destroy at this point, so call them all, even if one fails
* along the way.
*/
if (H5_UNLIKELY(H5TS_mutex_destroy(&lock->mutex)))
if (H5_UNLIKELY(H5TS_mutex_destroy(&lock->mutex) < 0))
ret_value = FAIL;
if (H5_UNLIKELY(H5TS_cond_destroy(&lock->cond_var)))
if (H5_UNLIKELY(H5TS_cond_destroy(&lock->cond_var) < 0))
ret_value = FAIL;

done:
if (have_mutex)
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex)))
if (H5_UNLIKELY(have_mutex))
if (H5_UNLIKELY(H5TS_mutex_unlock(&lock->mutex) < 0))
ret_value = FAIL;

FUNC_LEAVE_NOAPI_NAMECHECK_ONLY(ret_value)
Expand Down
22 changes: 14 additions & 8 deletions src/H5TSint.c
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ H5TS_api_lock(void)
HGOTO_DONE(FAIL);

/* Acquire the "attempt" mutex, increment the attempt lock count, release the lock */
if (H5_UNLIKELY(H5TS_mutex_lock(&H5TS_api_info_p.attempt_mutex)))
if (H5_UNLIKELY(H5TS_mutex_lock(&H5TS_api_info_p.attempt_mutex) < 0))
HGOTO_DONE(FAIL);
H5TS_api_info_p.attempt_lock_count++;
if (H5_UNLIKELY(H5TS_mutex_unlock(&H5TS_api_info_p.attempt_mutex)))
if (H5_UNLIKELY(H5TS_mutex_unlock(&H5TS_api_info_p.attempt_mutex) < 0))
HGOTO_DONE(FAIL);

/* Acquire the library's exclusive API lock */
Expand Down Expand Up @@ -300,7 +300,8 @@ H5TS__tinfo_create(void)
/* Note: Must use lock here also, since 'destroy' callback can be
* invoked asynchronously when a thread is joined.
*/
H5TS_mutex_lock(&H5TS_tinfo_mtx_s);
if (H5_UNLIKELY(H5TS_mutex_lock(&H5TS_tinfo_mtx_s) < 0))
HGOTO_DONE(NULL);

/* Reuse an info struct that's on the free list if possible */
if (NULL != (tinfo_node = H5TS_tinfo_next_free_s))
Expand All @@ -314,7 +315,8 @@ H5TS__tinfo_create(void)
new_id = ++H5TS_next_thrd_id_s;

/* Release the lock for modifying the thread info globals */
H5TS_mutex_unlock(&H5TS_tinfo_mtx_s);
if (H5_UNLIKELY(H5TS_mutex_unlock(&H5TS_tinfo_mtx_s) < 0))
HGOTO_DONE(NULL);

/* If a new info record is needed, allocate it */
if (NULL == tinfo_node) {
Expand Down Expand Up @@ -500,21 +502,25 @@ H5TS__tinfo_term(void)
FUNC_ENTER_PACKAGE_NAMECHECK_ONLY

/* Release nodes on the free list */
H5TS_mutex_lock(&H5TS_tinfo_mtx_s);
if (H5_UNLIKELY(H5TS_mutex_lock(&H5TS_tinfo_mtx_s) < 0))
HGOTO_DONE(FAIL);
while (H5TS_tinfo_next_free_s) {
H5TS_tinfo_node_t *next = H5TS_tinfo_next_free_s->next;
H5MM_free(H5TS_tinfo_next_free_s);
H5TS_tinfo_next_free_s = next;
}
H5TS_mutex_unlock(&H5TS_tinfo_mtx_s);
if (H5_UNLIKELY(H5TS_mutex_unlock(&H5TS_tinfo_mtx_s) < 0))
HGOTO_DONE(FAIL);

/* Release critical section / mutex for modifying the thread info globals */
H5TS_mutex_destroy(&H5TS_tinfo_mtx_s);
if (H5_UNLIKELY(H5TS_mutex_destroy(&H5TS_tinfo_mtx_s) < 0))
HGOTO_DONE(FAIL);

/* Release key for thread-specific API contexts */
if (H5_UNLIKELY(H5TS_key_delete(H5TS_thrd_info_key_g) < 0))
ret_value = FAIL;
HGOTO_DONE(FAIL);

done:
FUNC_LEAVE_NOAPI_NAMECHECK_ONLY(ret_value)
} /* end H5TS__tinfo_term() */

Expand Down
Loading

0 comments on commit d78bc6b

Please sign in to comment.