From e56811ed4db61ac2ac14d28a7a8fac83f41c43b8 Mon Sep 17 00:00:00 2001 From: Gustavo Sverzut Barbieri Date: Thu, 18 Aug 2016 13:36:05 -0300 Subject: [PATCH] eina_error: allow errno.h codes. we have some duplication of errors between Eina_Error and errno.h, however we should use Eina_Error to extend the traditional errno.h system. then change eina_error_msg_register() and eina_error_msg_static_register() to return a magic bit to state the number was registered, and on other functions test this bit in order to operate on registered values, otherwise fallback to errno.h, such as strerror(). It also deprecates 2 clear duplicated errors: - EINA_ERROR_OUT_OF_MEMORY -> ENOMEM - EINA_ERROR_TIMEOUT -> ETIMEDOUT There are two details when using strerror(): - old behavior did not return strings for non-error, such as "Success" or "Unknown error ${N}" - thread-safety issues: since we must be thread safe, then use strerror_r() and eina_stringshare_add() that value, keeping a hash of cached values --- src/lib/eina/eina_error.c | 128 +++++++++++++++++++++++--- src/lib/eina/eina_error.h | 56 +++++++++-- src/lib/eina/eina_inline_lock_posix.x | 2 +- src/lib/eina/eina_lock.h | 6 +- src/tests/ecore/ecore_test_promise.c | 4 +- src/tests/eina/eina_test_error.c | 25 ++++- src/tests/eina/eina_test_lock.c | 2 +- 7 files changed, 194 insertions(+), 29 deletions(-) diff --git a/src/lib/eina/eina_error.c b/src/lib/eina/eina_error.c index 3e47082032..85cf708502 100644 --- a/src/lib/eina/eina_error.c +++ b/src/lib/eina/eina_error.c @@ -38,6 +38,9 @@ #include "eina_error.h" #include "eina_stringshare.h" #include "eina_lock.h" +#ifdef EINA_HAVE_THREADS +#include "eina_hash.h" +#endif /* TODO * + add a wrapper for assert? @@ -61,10 +64,21 @@ struct _Eina_Error_Message const char *string; }; +#ifdef EINA_HAVE_THREADS +static Eina_Spinlock _eina_errno_msgs_lock; +static Eina_Hash *_eina_errno_msgs = NULL; +#endif static Eina_Error_Message *_eina_errors = NULL; static size_t _eina_errors_count = 0; static size_t _eina_errors_allocated = 0; +/* used to differentiate registered errors from errno.h */ +#define EINA_ERROR_REGISTERED_BIT (1 << 30) +#define EINA_ERROR_REGISTERED_CHECK(err) ((err) & EINA_ERROR_REGISTERED_BIT) + +#define EINA_ERROR_FROM_INDEX(idx) ((idx) | EINA_ERROR_REGISTERED_BIT) +#define EINA_ERROR_TO_INDEX(err) ((err) & (~EINA_ERROR_REGISTERED_BIT)) + static Eina_Error _eina_last_error; static Eina_TLS _eina_last_key; @@ -109,11 +123,9 @@ _eina_error_msg_alloc(void) * @cond LOCAL */ -EAPI Eina_Error EINA_ERROR_OUT_OF_MEMORY = 0; -static const char EINA_ERROR_OUT_OF_MEMORY_STR[] = "Out of memory"; +EAPI Eina_Error EINA_ERROR_OUT_OF_MEMORY = ENOMEM; -EWAPI Eina_Error EINA_ERROR_TIMEOUT = 0; -static const char EINA_ERROR_TIMEOUT_STR[] = "Timed out"; +EWAPI Eina_Error EINA_ERROR_TIMEOUT = ETIMEDOUT; /** * @endcond @@ -135,13 +147,25 @@ static const char EINA_ERROR_TIMEOUT_STR[] = "Timed out"; Eina_Bool eina_error_init(void) { - /* TODO register the eina's basic errors */ - EINA_ERROR_OUT_OF_MEMORY = eina_error_msg_static_register( - EINA_ERROR_OUT_OF_MEMORY_STR); - EINA_ERROR_TIMEOUT = eina_error_msg_static_register(EINA_ERROR_TIMEOUT_STR); if (!eina_tls_new(&_eina_last_key)) return EINA_FALSE; + +#ifdef EINA_HAVE_THREADS + if (!eina_spinlock_new(&_eina_errno_msgs_lock)) goto failed_lock; + _eina_errno_msgs = eina_hash_int32_new(EINA_FREE_CB(eina_stringshare_del)); + if (!_eina_errno_msgs) goto failed_hash; +#endif + return EINA_TRUE; + +#ifdef EINA_HAVE_THREADS + failed_hash: + eina_spinlock_free(&_eina_errno_msgs_lock); + failed_lock: + eina_tls_free(_eina_last_key); + _eina_last_error = 0; + return EINA_FALSE; +#endif } /** @@ -172,6 +196,12 @@ eina_error_shutdown(void) _eina_errors_count = 0; _eina_errors_allocated = 0; +#ifdef EINA_HAVE_THREADS + eina_hash_free(_eina_errno_msgs); + _eina_errno_msgs = NULL; + eina_spinlock_free(&_eina_errno_msgs_lock); +#endif + eina_tls_free(_eina_last_key); _eina_last_error = 0; @@ -201,7 +231,7 @@ eina_error_msg_register(const char *msg) return 0; } - return _eina_errors_count; /* identifier = index + 1 (== _count). */ + return EINA_ERROR_FROM_INDEX(_eina_errors_count); /* identifier = index + 1 (== _count). */ } EAPI Eina_Error @@ -217,13 +247,15 @@ eina_error_msg_static_register(const char *msg) eem->string_allocated = EINA_FALSE; eem->string = msg; - return _eina_errors_count; /* identifier = index + 1 (== _count). */ + return EINA_ERROR_FROM_INDEX(_eina_errors_count); /* identifier = index + 1 (== _count). */ } EAPI Eina_Bool eina_error_msg_modify(Eina_Error error, const char *msg) { EINA_SAFETY_ON_NULL_RETURN_VAL(msg, EINA_FALSE); + EINA_SAFETY_ON_FALSE_RETURN_VAL(EINA_ERROR_REGISTERED_CHECK(error), EINA_FALSE); + error = EINA_ERROR_TO_INDEX(error); if (error < 1) return EINA_FALSE; @@ -249,6 +281,72 @@ eina_error_msg_modify(Eina_Error error, const char *msg) EAPI const char * eina_error_msg_get(Eina_Error error) { + if (!EINA_ERROR_REGISTERED_CHECK(error)) + { + const char unknown_prefix[] = "Unknown error "; + const char *msg; + + /* original behavior of this function did not return strings + * for unknown errors, so skip 0 ("Success") and + * "Unknown error $N". + */ + if (error == 0) return NULL; + +#ifndef EINA_HAVE_THREADS + msg = strerror(error); + if (strncmp(msg, unknown_prefix, sizeof(unknown_prefix) -1) == 0) + msg = NULL; +#else /* EINA_HAVE_THREADS */ + /* strerror() is not thread safe, so use a local buffer with + * strerror_r() and cache resolved strings in a hash so we can + * return the stringshared refernece. + */ + if (eina_spinlock_take(&_eina_errno_msgs_lock) != EINA_LOCK_SUCCEED) + { + EINA_SAFETY_ERROR("could not take spinlock for errno messages hash!"); + return NULL; + } + msg = eina_hash_find(_eina_errno_msgs, &error); + eina_spinlock_release(&_eina_errno_msgs_lock); + + if (!msg) + { + char buf[256] = ""; + const char *str; + +#if (_POSIX_C_SOURCE >= 200112L) && ! _GNU_SOURCE + if (strerror_r(error, buf, sizeof(buf)) == 0) /* XSI */ + str = buf; + else + str = NULL; +#else + str = strerror_r(error, buf, sizeof(buf)); /* GNU */ +#endif + if (!str) + EINA_SAFETY_ERROR("strerror_r() failed"); + else + { + if (strncmp(str, unknown_prefix, sizeof(unknown_prefix) -1) == 0) + msg = NULL; + else + { + msg = eina_stringshare_add(str); + if (eina_spinlock_take(&_eina_errno_msgs_lock) != EINA_LOCK_SUCCEED) + { + EINA_SAFETY_ERROR("could not take spinlock for errno messages hash!"); + return NULL; + } + eina_hash_add(_eina_errno_msgs, &error, msg); + eina_spinlock_release(&_eina_errno_msgs_lock); + } + } + } +#endif + return msg; + } + + error = EINA_ERROR_TO_INDEX(error); + if (error < 1) return NULL; @@ -288,10 +386,16 @@ eina_error_find(const char *msg) if (_eina_errors[i].string_allocated) { if (_eina_errors[i].string == msg) - return i + 1; + return EINA_ERROR_FROM_INDEX(i + 1); } if (!strcmp(_eina_errors[i].string, msg)) - return i + 1; + return EINA_ERROR_FROM_INDEX(i + 1); } + + /* not bothering to lookup errno.h as we don't have a "maximum + * error", thus we'd need to loop up to some arbitrary constant and + * keep comparing if strerror() returns something meaningful. + */ + return 0; } diff --git a/src/lib/eina/eina_error.h b/src/lib/eina/eina_error.h index a49aea581d..71df4848db 100644 --- a/src/lib/eina/eina_error.h +++ b/src/lib/eina/eina_error.h @@ -29,7 +29,7 @@ * * @section tutorial_error_registering_msg Registering messages * - * The error module can provide a system that mimics the errno system + * The error module can provide a system that extends the errno system * of the C standard library. It consists in 2 parts: * * @li A way of registering new messages with @@ -57,10 +57,20 @@ * * @brief This group discusses the functions that provide error management for projects. * - * The Eina error module provides a way to manage errors in a simple but - * powerful way in libraries and modules. It is also used in Eina itself. - * Similar to libC's @c errno and strerror() facilities, this is extensible and - * recommended for other libraries and applications as well. + * The Eina error module provides a way to manage errors in a simple + * but powerful way in libraries and modules. It is also used in Eina + * itself. An extension to libC's @c errno and strerror() facilities, + * this is extensible and recommended for other libraries and + * applications as well. + * + * While libC's @c errno is fixed, Eina_Error can be extended by + * libraries and applications with domain-specific codes and + * associated error messages. All @c errno.h codes are usable + * seamlessly with Eina_Error. The constants defined in errno.h will + * have messages as reported by strerror() in eina_error_msg_get() + * + * Success (no-error) condition is reported with Eina_Error 0 + * (#EINA_ERROR_NO_ERROR). * * A simple example of how to use this can be seen @ref tutorial_error_page * "here". @@ -71,14 +81,29 @@ /** * @typedef Eina_Error * @brief The integer type containing the error type. + * + * Errors are registered with eina_error_msg_register(), + * eina_error_msg_static_register() or those inherited from the + * system's @c errno.h such as @c ENOMEM. */ typedef int Eina_Error; +/** + * @def EINA_ERROR_NO_ERROR + * @brief No error reported. + * + * This is a convenience macro for people that are extra verbose, same + * as "0". + */ +#define EINA_ERROR_NO_ERROR ((Eina_Error)0) + /** * @var EINA_ERROR_OUT_OF_MEMORY * @brief The error identifier corresponding to lack of memory. + * + * @deprecated since 1.19, same as @c ENOMEM from @c errno.h */ -EAPI extern Eina_Error EINA_ERROR_OUT_OF_MEMORY; +EAPI extern Eina_Error EINA_ERROR_OUT_OF_MEMORY EINA_DEPRECATED; /* use ENOMEM */ /** * @brief Registers a new error type. @@ -92,6 +117,9 @@ EAPI extern Eina_Error EINA_ERROR_OUT_OF_MEMORY; * to @c 1. The description can be retrieved later by passing * the returned value to eina_error_msg_get(). * + * @note There is no need to register messages that exist in libC's @c + * errno.h, such as @c ENOMEM or @c EBADF. + * * @see eina_error_msg_static_register() */ EAPI Eina_Error eina_error_msg_register(const char *msg) EINA_ARG_NONNULL(1) EINA_WARN_UNUSED_RESULT; @@ -109,6 +137,9 @@ EAPI Eina_Error eina_error_msg_register(const char *msg) EINA_ARG_NONNULL(1) EI * to @c 1. The description can be retrieved later by passing * the returned value to eina_error_msg_get(). * + * @note There is no need to register messages that exist in libC's @c + * errno.h, such as @c ENOMEM or @c EBADF. + * * @see eina_error_msg_register() */ EAPI Eina_Error eina_error_msg_static_register(const char *msg) EINA_ARG_NONNULL(1) EINA_WARN_UNUSED_RESULT; @@ -127,6 +158,10 @@ EAPI Eina_Error eina_error_msg_static_register(const char *msg) EINA_ARG_NONNUL * then the string is not duplicated, otherwise the previous message * is unrefed and @p msg is copied. * + * @note It is not possible to modify messages that exist in libC's @c + * errno.h, such as @c ENOMEM or @c EBADF. + * + * * @see eina_error_msg_register() */ EAPI Eina_Bool eina_error_msg_modify(Eina_Error error, @@ -135,7 +170,7 @@ EAPI Eina_Bool eina_error_msg_modify(Eina_Error error, /** * @brief Returns the last set error. * - * @return The last error + * @return The last error or 0 (#EINA_ERROR_NO_ERROR). * * @details This function returns the last error set by eina_error_set(). The * description of the message is returned by eina_error_msg_get(). @@ -153,7 +188,7 @@ EAPI Eina_Error eina_error_get(void); * retrieved by eina_error_get(). * * @note This is also used to clear previous errors, in which case @p err should - * be @c 0. + * be @c 0 (#EINA_ERROR_NO_ERROR). * * @note This function is thread safe @since 1.10, but slower to use. */ @@ -179,6 +214,11 @@ EAPI const char *eina_error_msg_get(Eina_Error error) EINA_PURE; * * @details This function attempts to match @p msg with its corresponding #Eina_Error value. * If no such value is found, @c 0 is returned. + * + * @note this function only works for explicitly registered errors + * such as the messages given to eina_error_msg_register(), + * eina_error_msg_static_register() or modified with + * eina_error_msg_modify(). */ EAPI Eina_Error eina_error_find(const char *msg) EINA_ARG_NONNULL(1) EINA_PURE; diff --git a/src/lib/eina/eina_inline_lock_posix.x b/src/lib/eina/eina_inline_lock_posix.x index 70375abe36..1b9e73fb8f 100644 --- a/src/lib/eina/eina_inline_lock_posix.x +++ b/src/lib/eina/eina_inline_lock_posix.x @@ -521,7 +521,7 @@ eina_condition_timedwait(Eina_Condition *cond, double t) else if (err == ETIMEDOUT) { r = EINA_FALSE; - if (&EINA_ERROR_TIMEOUT) eina_error_set(EINA_ERROR_TIMEOUT); + eina_error_set(ETIMEDOUT); } else EINA_LOCK_ABORT_DEBUG(err, cond_timedwait, cond); diff --git a/src/lib/eina/eina_lock.h b/src/lib/eina/eina_lock.h index ed79649982..4db5a84f9e 100644 --- a/src/lib/eina/eina_lock.h +++ b/src/lib/eina/eina_lock.h @@ -100,8 +100,10 @@ typedef void (*Eina_TLS_Delete_Cb)(void *ptr); /** * @brief Error set when a blocking function timed out. * @since 1.19 + * + * @deprecated Use ETIMEDOUT instead. */ -EWAPI extern Eina_Error EINA_ERROR_TIMEOUT; +EWAPI extern Eina_Error EINA_ERROR_TIMEOUT EINA_DEPRECATED; /* use ETIMEDOUT */ # include "eina_inline_lock_posix.x" @@ -257,7 +259,7 @@ static inline Eina_Bool eina_condition_wait(Eina_Condition *cond); * @param[in] t The maximum amount of time to wait, in seconds. * * @return #EINA_TRUE on success, #EINA_FALSE otherwise. If the operation - * timed out, eina error will be set to #EINA_ERROR_TIMEOUT. + * timed out, eina error will be set to #ETIMEDOUT. * * @details This function makes a thread block until either a signal is sent to it via * @p cond or @p t seconds have passed. diff --git a/src/tests/ecore/ecore_test_promise.c b/src/tests/ecore/ecore_test_promise.c index 3c7d8ebd8c..50df5083b3 100644 --- a/src/tests/ecore/ecore_test_promise.c +++ b/src/tests/ecore/ecore_test_promise.c @@ -33,12 +33,12 @@ END_TEST void promise_error_thread(const void* data EINA_UNUSED, Eina_Promise_Owner* promise, Ecore_Thread* thread EINA_UNUSED) { - eina_promise_owner_error_set(promise, EINA_ERROR_OUT_OF_MEMORY); + eina_promise_owner_error_set(promise, ENOMEM); } void promise_error_callback(void* data EINA_UNUSED, Eina_Error error) { - ck_assert(error == EINA_ERROR_OUT_OF_MEMORY); + ck_assert(error == ENOMEM); ecore_main_loop_quit(); } diff --git a/src/tests/eina/eina_test_error.c b/src/tests/eina/eina_test_error.c index 1c27213822..b101dc8c2d 100644 --- a/src/tests/eina/eina_test_error.c +++ b/src/tests/eina/eina_test_error.c @@ -84,6 +84,10 @@ START_TEST(eina_error_errno) eina_error_set(test); fail_if(eina_error_get() != test); + eina_error_set(EBADF); + ck_assert_int_eq(eina_error_get(), EBADF); + ck_assert_str_eq(eina_error_msg_get(EBADF), strerror(EBADF)); + eina_shutdown(); } END_TEST @@ -186,6 +190,7 @@ END_TEST START_TEST(eina_error_test_failures) { struct log_ctx ctx; + Eina_Error local_error; eina_init(); @@ -206,17 +211,31 @@ START_TEST(eina_error_test_failures) ck_assert_int_eq(eina_error_msg_static_register(NULL), 0); fail_unless(ctx.did); + TEST_MAGIC_SAFETY("eina_error_msg_modify", + "safety check failed: EINA_ERROR_REGISTERED_CHECK(error) is false"); ck_assert_int_eq(eina_error_msg_modify(0, "X"), EINA_FALSE); + + TEST_MAGIC_SAFETY("eina_error_msg_modify", + "safety check failed: EINA_ERROR_REGISTERED_CHECK(error) is false"); ck_assert_int_eq(eina_error_msg_modify(4096, "X"), EINA_FALSE); TEST_MAGIC_SAFETY("eina_error_msg_modify", "safety check failed: msg == NULL"); - ck_assert_int_eq(eina_error_msg_modify(EINA_ERROR_OUT_OF_MEMORY, NULL), + ck_assert_int_eq(eina_error_msg_modify(ENOMEM, NULL), EINA_FALSE); fail_unless(ctx.did); - ck_assert_str_eq(eina_error_msg_get(EINA_ERROR_OUT_OF_MEMORY), - "Out of memory"); + local_error = eina_error_msg_static_register("Local error for test"); + ck_assert_int_ne(local_error, 0); + + TEST_MAGIC_SAFETY("eina_error_msg_modify", + "safety check failed: msg == NULL"); + ck_assert_int_eq(eina_error_msg_modify(local_error, NULL), + EINA_FALSE); + fail_unless(ctx.did); + + ck_assert_str_eq(eina_error_msg_get(ENOMEM), + "Cannot allocate memory"); TEST_MAGIC_SAFETY("eina_error_find", "safety check failed: msg == NULL"); diff --git a/src/tests/eina/eina_test_lock.c b/src/tests/eina/eina_test_lock.c index fb18f3b881..4435ddf5a0 100644 --- a/src/tests/eina/eina_test_lock.c +++ b/src/tests/eina/eina_test_lock.c @@ -227,7 +227,7 @@ START_TEST(eina_test_rwlock) delay = (ts2.tv_sec - ts.tv_sec) * 1000L + (ts2.tv_nsec - ts.tv_nsec) / 1000000L; fail_if(delay < 50); fail_if(delay > 200); - fail_if(eina_error_get() != EINA_ERROR_TIMEOUT); + fail_if(eina_error_get() != ETIMEDOUT); fail_if(eina_lock_release(&mtcond) != EINA_LOCK_SUCCEED); eina_thread_join(thread);