From 3b1c80fd0fe1d45d230c1f9a9d5602c2e21f0e0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 9 Sep 2022 20:25:47 +0200 Subject: [PATCH] Fix error reporting for POSIX Threads functions Commit 3608abc8fa6a33046e1d34a0789cf7c9547f09ad inadvertently carried over a mistake in logging pthread_cond_init() errors to the ERRNO_CHECK() preprocessor macro: instead of passing the value returned by a given pthread_*() function to strerror_r(), ERRNO_CHECK() passes the errno variable to strerror_r(). This causes bogus error reports because POSIX Threads API functions do not set the errno variable. Fix by passing the value returned by a given pthread_*() function instead of the errno variable to strerror_r(). Since this change makes the name of the affected macro (ERRNO_CHECK()) confusing, rename the latter to PTHREADS_RUNTIME_CHECK(). Also log the integer error value returned by a given pthread_*() function verbatim to rule out any further confusion in runtime error reporting. --- lib/isc/include/isc/condition.h | 40 ++++++++++++++++----------------- lib/isc/include/isc/mutex.h | 26 ++++++++++----------- lib/isc/include/isc/rwlock.h | 40 ++++++++++++++++----------------- lib/isc/include/isc/util.h | 18 +++++++++------ lib/isc/thread.c | 8 +++---- 5 files changed, 68 insertions(+), 64 deletions(-) diff --git a/lib/isc/include/isc/condition.h b/lib/isc/include/isc/condition.h index 445ba4d643..09302b35a9 100644 --- a/lib/isc/include/isc/condition.h +++ b/lib/isc/include/isc/condition.h @@ -60,34 +60,34 @@ typedef pthread_cond_t isc_condition_t; #endif /* ISC_TRACK_PTHREADS_OBJECTS */ -#define isc__condition_init(cond) \ - { \ - int _ret = pthread_cond_init(cond, NULL); \ - ERRNO_CHECK(pthread_cond_init, _ret); \ +#define isc__condition_init(cond) \ + { \ + int _ret = pthread_cond_init(cond, NULL); \ + PTHREADS_RUNTIME_CHECK(pthread_cond_init, _ret); \ } -#define isc__condition_wait(cp, mp) \ - { \ - int _ret = pthread_cond_wait(cp, mp); \ - ERRNO_CHECK(pthread_cond_wait, _ret); \ +#define isc__condition_wait(cp, mp) \ + { \ + int _ret = pthread_cond_wait(cp, mp); \ + PTHREADS_RUNTIME_CHECK(pthread_cond_wait, _ret); \ } -#define isc__condition_signal(cp) \ - { \ - int _ret = pthread_cond_signal(cp); \ - ERRNO_CHECK(pthread_cond_signal, _ret); \ +#define isc__condition_signal(cp) \ + { \ + int _ret = pthread_cond_signal(cp); \ + PTHREADS_RUNTIME_CHECK(pthread_cond_signal, _ret); \ } -#define isc__condition_broadcast(cp) \ - { \ - int _ret = pthread_cond_broadcast(cp); \ - ERRNO_CHECK(pthread_cond_broadcast, _ret); \ +#define isc__condition_broadcast(cp) \ + { \ + int _ret = pthread_cond_broadcast(cp); \ + PTHREADS_RUNTIME_CHECK(pthread_cond_broadcast, _ret); \ } -#define isc__condition_destroy(cp) \ - { \ - int _ret = pthread_cond_destroy(cp); \ - ERRNO_CHECK(pthread_cond_destroy, _ret); \ +#define isc__condition_destroy(cp) \ + { \ + int _ret = pthread_cond_destroy(cp); \ + PTHREADS_RUNTIME_CHECK(pthread_cond_destroy, _ret); \ } isc_result_t diff --git a/lib/isc/include/isc/mutex.h b/lib/isc/include/isc/mutex.h index 8c02d0f234..78aa153707 100644 --- a/lib/isc/include/isc/mutex.h +++ b/lib/isc/include/isc/mutex.h @@ -60,28 +60,28 @@ extern pthread_mutexattr_t isc__mutex_init_attr; #define isc__mutex_init(mp) \ { \ int _ret = pthread_mutex_init(mp, &isc__mutex_init_attr); \ - ERRNO_CHECK(pthread_mutex_init, _ret); \ + PTHREADS_RUNTIME_CHECK(pthread_mutex_init, _ret); \ } -#define isc__mutex_lock(mp) \ - { \ - int _ret = pthread_mutex_lock(mp); \ - ERRNO_CHECK(pthread_mutex_lock, _ret); \ +#define isc__mutex_lock(mp) \ + { \ + int _ret = pthread_mutex_lock(mp); \ + PTHREADS_RUNTIME_CHECK(pthread_mutex_lock, _ret); \ } -#define isc__mutex_unlock(mp) \ - { \ - int _ret = pthread_mutex_unlock(mp); \ - ERRNO_CHECK(pthread_mutex_unlock, _ret); \ +#define isc__mutex_unlock(mp) \ + { \ + int _ret = pthread_mutex_unlock(mp); \ + PTHREADS_RUNTIME_CHECK(pthread_mutex_unlock, _ret); \ } #define isc__mutex_trylock(mp) \ ((pthread_mutex_trylock(mp) == 0) ? ISC_R_SUCCESS : ISC_R_LOCKBUSY) -#define isc__mutex_destroy(mp) \ - { \ - int _ret = pthread_mutex_destroy(mp); \ - ERRNO_CHECK(pthread_mutex_destroy, _ret); \ +#define isc__mutex_destroy(mp) \ + { \ + int _ret = pthread_mutex_destroy(mp); \ + PTHREADS_RUNTIME_CHECK(pthread_mutex_destroy, _ret); \ } ISC_LANG_ENDDECLS diff --git a/lib/isc/include/isc/rwlock.h b/lib/isc/include/isc/rwlock.h index b6d5232f52..30bdd85763 100644 --- a/lib/isc/include/isc/rwlock.h +++ b/lib/isc/include/isc/rwlock.h @@ -127,34 +127,34 @@ typedef struct isc_rwlock isc__rwlock_t; #endif /* USE_PTHREAD_RWLOCK */ -#define isc__rwlock_init(rwl, rq, wq) \ - { \ - int _ret = isc___rwlock_init(rwl, rq, wq); \ - ERRNO_CHECK(isc___rwlock_init, _ret); \ +#define isc__rwlock_init(rwl, rq, wq) \ + { \ + int _ret = isc___rwlock_init(rwl, rq, wq); \ + PTHREADS_RUNTIME_CHECK(isc___rwlock_init, _ret); \ } -#define isc__rwlock_lock(rwl, type) \ - { \ - int _ret = isc___rwlock_lock(rwl, type); \ - ERRNO_CHECK(isc___rwlock_lock, _ret); \ +#define isc__rwlock_lock(rwl, type) \ + { \ + int _ret = isc___rwlock_lock(rwl, type); \ + PTHREADS_RUNTIME_CHECK(isc___rwlock_lock, _ret); \ } -#define isc__rwlock_unlock(rwl, type) \ - { \ - int _ret = isc___rwlock_unlock(rwl, type); \ - ERRNO_CHECK(isc___rwlock_unlock, _ret); \ +#define isc__rwlock_unlock(rwl, type) \ + { \ + int _ret = isc___rwlock_unlock(rwl, type); \ + PTHREADS_RUNTIME_CHECK(isc___rwlock_unlock, _ret); \ } -#define isc__rwlock_downgrade(rwl) \ - { \ - int _ret = isc___rwlock_downgrade(rwl); \ - ERRNO_CHECK(isc___rwlock_downgrade, _ret); \ +#define isc__rwlock_downgrade(rwl) \ + { \ + int _ret = isc___rwlock_downgrade(rwl); \ + PTHREADS_RUNTIME_CHECK(isc___rwlock_downgrade, _ret); \ } -#define isc__rwlock_destroy(rwl) \ - { \ - int _ret = isc___rwlock_destroy(rwl); \ - ERRNO_CHECK(isc___rwlock_destroy, _ret); \ +#define isc__rwlock_destroy(rwl) \ + { \ + int _ret = isc___rwlock_destroy(rwl); \ + PTHREADS_RUNTIME_CHECK(isc___rwlock_destroy, _ret); \ } int diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h index 243ded9759..9f7098c8c9 100644 --- a/lib/isc/include/isc/util.h +++ b/lib/isc/include/isc/util.h @@ -334,13 +334,17 @@ mock_assert(const int result, const char *const expression, #endif /* UNIT_TESTING */ -/*% Runtime check which logs the error string corresponding to errno */ -#define ERRNO_CHECK(func, ret) \ - if ((ret) != 0) { \ - char _strerrorbuf[ISC_STRERRORSIZE]; \ - strerror_r(errno, _strerrorbuf, sizeof(_strerrorbuf)); \ - isc_error_fatal(__FILE__, __LINE__, "%s() failed in %s(): %s", \ - #func, __func__, _strerrorbuf); \ +/*% + * Runtime check which logs the error value returned by a POSIX Threads + * function and the error string that corresponds to it + */ +#define PTHREADS_RUNTIME_CHECK(func, ret) \ + if ((ret) != 0) { \ + char _strerrorbuf[ISC_STRERRORSIZE]; \ + strerror_r(ret, _strerrorbuf, sizeof(_strerrorbuf)); \ + isc_error_fatal(__FILE__, __LINE__, \ + "%s(): %s() failed with error %d (%s)", \ + __func__, #func, ret, _strerrorbuf); \ } /*% diff --git a/lib/isc/thread.c b/lib/isc/thread.c index fef4998dab..a9df98a6a2 100644 --- a/lib/isc/thread.c +++ b/lib/isc/thread.c @@ -58,18 +58,18 @@ isc_thread_create(isc_threadfunc_t func, isc_threadarg_t arg, #if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \ defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) ret = pthread_attr_getstacksize(&attr, &stacksize); - ERRNO_CHECK(pthread_attr_getstacksize, ret); + PTHREADS_RUNTIME_CHECK(pthread_attr_getstacksize, ret); if (stacksize < THREAD_MINSTACKSIZE) { ret = pthread_attr_setstacksize(&attr, THREAD_MINSTACKSIZE); - ERRNO_CHECK(pthread_attr_setstacksize, ret); + PTHREADS_RUNTIME_CHECK(pthread_attr_setstacksize, ret); } #endif /* if defined(HAVE_PTHREAD_ATTR_GETSTACKSIZE) && \ * defined(HAVE_PTHREAD_ATTR_SETSTACKSIZE) */ ret = pthread_create(thread, &attr, isc__trampoline_run, trampoline_arg); - ERRNO_CHECK(pthread_create, ret); + PTHREADS_RUNTIME_CHECK(pthread_create, ret); pthread_attr_destroy(&attr); @@ -80,7 +80,7 @@ void isc_thread_join(isc_thread_t thread, isc_threadresult_t *result) { int ret = pthread_join(thread, result); - ERRNO_CHECK(pthread_join, ret); + PTHREADS_RUNTIME_CHECK(pthread_join, ret); } void