From 160b3df5176400f604d59ef676b80fe297d6d3d5 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Mon, 29 Oct 2018 10:52:02 -0400 Subject: [PATCH 01/20] refactor error_state to be fixed sized and API to avoid memory allocation Signed-off-by: William Woodall --- CMakeLists.txt | 12 +- include/rcutils/error_handling.h | 188 ++++++++------ src/error_handling.c | 359 ++++++++------------------- src/error_handling_helpers.h | 160 ++++++++++++ test/test_error_handling.cpp | 193 +++++++++++--- test/test_error_handling_helpers.cpp | 143 +++++++++++ 6 files changed, 684 insertions(+), 371 deletions(-) create mode 100644 src/error_handling_helpers.h create mode 100644 test/test_error_handling_helpers.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8624508f..7e0cc441 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -202,9 +202,19 @@ if(BUILD_TESTING) rcutils_custom_add_gmock(test_error_handling test/test_error_handling.cpp # Append the directory of librcutils so it is found at test time. APPEND_LIBRARY_DIRS "$" + ENV ${memory_tools_test_env_vars} ) if(TARGET test_error_handling) - target_link_libraries(test_error_handling ${PROJECT_NAME}) + target_link_libraries(test_error_handling ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools) + endif() + + rcutils_custom_add_gmock(test_error_handling_helpers test/test_error_handling_helpers.cpp + # Append the directory of librcutils so it is found at test time. + APPEND_LIBRARY_DIRS "$" + ENV ${memory_tools_test_env_vars} + ) + if(TARGET test_error_handling_helpers) + target_link_libraries(test_error_handling_helpers osrf_testing_tools_cpp::memory_tools) endif() rcutils_custom_add_gtest(test_split diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index 1bb24092..8177ab6d 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -22,6 +22,8 @@ extern "C" { #endif +#define __STDC_WANT_LIB_EXT1__ 1 // indicate we would like strnlen_s if available +#include #include #include #include @@ -29,22 +31,12 @@ extern "C" #include #include "rcutils/allocator.h" -#include "rcutils/format_string.h" #include "rcutils/macros.h" +#include "rcutils/snprintf.h" #include "rcutils/types/rcutils_ret.h" #include "rcutils/visibility_control.h" -/// Struct which encapsulates the error state set by RCUTILS_SET_ERROR_MSG(). -typedef struct rcutils_error_state_t -{ - const char * message; - const char * file; - size_t line_number; - rcutils_allocator_t allocator; -} rcutils_error_state_t; - -// TODO(dhood): use __STDC_LIB_EXT1__ if/when supported in other implementations. -#if defined(_WIN32) +#ifdef __STDC_LIB_EXT1__ // Limit the buffer size in the `fwrite` call to give an upper bound to buffer overrun in the case // of non-null terminated `msg`. #define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) fwrite(msg, sizeof(char), strnlen_s(msg, 4096), stderr) @@ -52,28 +44,95 @@ typedef struct rcutils_error_state_t #define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) fwrite(msg, sizeof(char), strlen(msg), stderr) #endif -/// Copy an error state into a destination error state. +// fixed constraints +#define RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH 20 // "18446744073709551615" +#define RCUTILS_ERROR_FORMATTING_CHARACTERS 6 // ', at ' + ':' + +// max formatted string length +#define RCUTILS_ERROR_MESSAGE_MAX_LENGTH 1024 + +// adjustable max length for user defined error message +// remember "chained" errors will include previously specified file paths +// e.g. "some error, at /path/to/a.c:42, at /path/to/b.c:42" +#define RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH 768 +// with RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH = 768, RCUTILS_ERROR_STATE_FILE_MAX_LENGTH == 228 +#define RCUTILS_ERROR_STATE_FILE_MAX_LENGTH ( \ + RCUTILS_ERROR_MESSAGE_MAX_LENGTH - \ + RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH - \ + RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH - \ + RCUTILS_ERROR_FORMATTING_CHARACTERS - \ + 1) + +/// Struct wrapping a fixed-size c string used for returning the formatted error string. +typedef struct rcutils_error_string_t +{ + const char str[RCUTILS_ERROR_MESSAGE_MAX_LENGTH]; +} rcutils_error_string_t; + +/// Struct which encapsulates the error state set by RCUTILS_SET_ERROR_MSG(). +typedef struct rcutils_error_state_t +{ + /// User message storage, limited to RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH characters. + const char message[RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH]; + /// File name, limited to what's left from RCUTILS_ERROR_STATE_MAX_SIZE characters + /// after subtracting storage for others. + const char file[RCUTILS_ERROR_STATE_FILE_MAX_LENGTH]; + /// Line number of error. + uint64_t line_number; +} rcutils_error_state_t; + +// make sure our math is right... +static_assert( + RCUTILS_ERROR_MESSAGE_MAX_LENGTH == ( + RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH + + RCUTILS_ERROR_STATE_FILE_MAX_LENGTH + + RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH + + RCUTILS_ERROR_FORMATTING_CHARACTERS + + 1 /* null terminating character */), + "Maximum length calculations incorrect"); + +/// Forces initialization of thread-local storage if called in a newly created thread. /** - * The destination state must be empty, the memory will not be free'd. - * The allocator from the source is used to allocate memory in the dst. + * If this function is not called beforehand, then the first time the error + * state is set or the first time the error message is retrieved, the default + * allocator will be used to allocate thread-local storage. + * + * This function may or may not allocate memory. + * The system's thread-local storage implementation may need to + * allocate memory (usually no way of knowing how much storage is needed if you + * cannot know how many threads will be created). + * Most implementations (e.g. C11, C++11, and pthread) do not allow you to + * specify how this memory is allocated, but if the implementation allows, the + * given allocator to this function will be used, otherwise it is unused. + * This isn't typically an issue since the memory is only free'd on thread + * destruction, and people trying to avoid memory allocation will also be + * avoiding thread creation and destruction. + * + * It is worth considering that repeated thread creation and destruction will + * result in repeated memory allocations and could result in memory + * fragmentation. + * This is typically avoided anyways by using pools of threads. * - * The copied error_state should be finalized with rcutils_error_state_fini(). + * In case an error is indicated by the return code, no error message will have + * been set. * - * \param[in] src the error state to copy from - * \param[out] dst the error state to copy into - * \returns RCUTILS_RET_OK if successful, or - * \returns RCUTILS_RET_BAD_ALLOC if memory allocation fails, or - * \returns RCUTILS_RET_ERROR if an unknown error occurs. + * If called more than once in a thread, or after implicitly initialized by + * setting the error state, it will still return `RCUTILS_RET_OK`, and even + * if the given allocator is invalid. + * Essentially this function does nothing if thread-local storage has already + * been called. + * If already initialized, the given allocator is ignored, even if it does not + * match the allocator used originally to initialize the thread-local storage. + * + * \return `RCUTILS_RET_OK` if successful, or + * \return `RCUTILS_RET_INVALID_ARGUMENT` if the allocator is invalid, or + * \return `RCUTILS_RET_BAD_ALLOC` if allocating memory fails, or + * \return `RCUTILS_RET_ERROR` if an unspecified error occurs. */ RCUTILS_PUBLIC RCUTILS_WARN_UNUSED rcutils_ret_t -rcutils_error_state_copy(const rcutils_error_state_t * src, rcutils_error_state_t * dst); - -/// Finalizes a copied error state. -RCUTILS_PUBLIC -void -rcutils_error_state_fini(rcutils_error_state_t * error_state); +rcutils_initialize_error_handling_thread_local_storage(rcutils_allocator_t allocator); /// Set the error message, as well as the file and line on which it occurred. /** @@ -82,25 +141,16 @@ rcutils_error_state_fini(rcutils_error_state_t * error_state); * * The error_msg parameter is copied into the internal error storage and must * be null terminated. - * The file parameter is not copied, but instead is assumed to be a global as - * it should be set to the __FILE__ preprocessor literal when used with the - * RCUTILS_SET_ERROR_MSG() macro. - * It should also be null terminated. - * - * The allocator is kept within the error state so that it can be used to - * deallocate it in the future. - * Therefore the allocator state needs to exist until after the last time - * rcutils_reset_error() is called. + * The file parameter is copied into the internal error storage and must + * be null terminated. * * \param[in] error_string The error message to set. * \param[in] file The path to the file in which the error occurred. * \param[in] line_number The line number on which the error occurred. - * \param[in] allocator The allocator to be used when allocating space for the error state. */ RCUTILS_PUBLIC void -rcutils_set_error_state( - const char * error_string, const char * file, size_t line_number, rcutils_allocator_t allocator); +rcutils_set_error_state(const char * error_string, const char * file, size_t line_number); /// Check an argument for a null value. /** @@ -109,11 +159,10 @@ rcutils_set_error_state( * * \param[in] argument The argument to test. * \param[in] error_return_type The type to return if the argument is `NULL`. - * \param[in] allocator The allocator to use if an error message needs to be allocated. */ -#define RCUTILS_CHECK_ARGUMENT_FOR_NULL(argument, error_return_type, allocator) \ +#define RCUTILS_CHECK_ARGUMENT_FOR_NULL(argument, error_return_type) \ RCUTILS_CHECK_FOR_NULL_WITH_MSG(argument, #argument " argument is null", \ - return error_return_type, allocator) + return error_return_type) /// Check a value for null, with an error message and error statement. /** @@ -123,11 +172,10 @@ rcutils_set_error_state( * \param[in] value The value to test. * \param[in] msg The error message if `value` is `NULL`. * \param[in] error_statement The statement to evaluate if `value` is `NULL`. - * \param[in] allocator The allocator to use if an error message needs to be allocated. */ -#define RCUTILS_CHECK_FOR_NULL_WITH_MSG(value, msg, error_statement, allocator) \ +#define RCUTILS_CHECK_FOR_NULL_WITH_MSG(value, msg, error_statement) \ if (NULL == value) { \ - RCUTILS_SET_ERROR_MSG(msg, allocator); \ + RCUTILS_SET_ERROR_MSG(msg); \ error_statement; \ } @@ -140,34 +188,32 @@ rcutils_set_error_state( * also thread local. * * \param[in] msg The error message to be set. - * \param[in] allocator The allocator to be used when allocating space for the error state. */ -#define RCUTILS_SET_ERROR_MSG(msg, allocator) \ - rcutils_set_error_state(msg, __FILE__, __LINE__, allocator); +#define RCUTILS_SET_ERROR_MSG(msg) rcutils_set_error_state(msg, __FILE__, __LINE__); /// Set the error message using a format string and format arguments. /** - * This function sets the error message using the given format string and - * then frees the memory allocated during the string formatting. + * This function sets the error message using the given format string. + * The resulting formatted string is silently truncated at + * RCUTILS_ERROR_MESSAGE_MAX_LENGTH. * - * \param[in] allocator The allocator to be used when allocating space for the error state. * \param[in] format_string The string to be used as the format of the error message. * \param[in] ... Arguments for the format string. */ -#define RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(allocator, format_string, ...) \ +#define RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(format_string, ...) \ do { \ - char * output_msg = NULL; \ - output_msg = rcutils_format_string(allocator, format_string, __VA_ARGS__); \ - if (output_msg) { \ - RCUTILS_SET_ERROR_MSG(output_msg, allocator); \ - allocator.deallocate(output_msg, allocator.state); \ + char output_msg[RCUTILS_ERROR_MESSAGE_MAX_LENGTH]; \ + int ret = rcutils_snprintf(output_msg, sizeof(output_msg), format_string, __VA_ARGS__); \ + if (ret < 0) { \ + RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to call snprintf for error message formatting\n"); \ } else { \ - RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to allocate memory for error message\n"); \ + RCUTILS_SET_ERROR_MSG(output_msg); \ } \ } while (false) /// Return `true` if the error is set, otherwise `false`. RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED bool rcutils_error_is_set(void); @@ -181,32 +227,24 @@ rcutils_error_is_set(void); * \return A pointer to the current error state struct. */ RCUTILS_PUBLIC +RCUTILS_WARN_UNUSED const rcutils_error_state_t * rcutils_get_error_state(void); -/// Return the error message followed by `, at :`, or `NULL`. -/** - * The returned pointer is valid until RCUTILS_SET_ERROR_MSG(), - * rcutils_set_error_state(), or rcutils_reset_error() are called from the same thread. - * - * \return The current formatted error string, or NULL if not set. - */ -RCUTILS_PUBLIC -const char * -rcutils_get_error_string(void); - /// Return the error message followed by `, at :` if set, else "error not set". /** - * This function is guaranteed to return a valid c-string. - * - * The returned pointer is valid until RCUTILS_SET_ERROR_MSG, - * rcutils_set_error_state, or rcutils_reset_error are called in the same thread. + * This function is "safe" because it returns a copy of the current error + * string or one containing the string "error not set" if no error was set. + * This ensures your copy is owned by you and is never invalidated by error + * handling calls, and that the c string inside is always valid and null + * terminated. * * \return The current error string, with file and line number, or "error not set" if not set. */ RCUTILS_PUBLIC -const char * -rcutils_get_error_string_safe(void); +RCUTILS_WARN_UNUSED +rcutils_error_string_t +rcutils_get_error_string(void); /// Reset the error state by clearing any previously set error state. RCUTILS_PUBLIC diff --git a/src/error_handling.c b/src/error_handling.c index efdf8f65..b4a95bfc 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -14,313 +14,150 @@ // Note: migrated from rmw/error_handling.c in 2017-04 +#include + #include #include #include #include -#include +#include #include #include -// When this define evaluates to true (default), then messages will printed to -// stderr when an error is encoutered while setting the error state. -// For example, when memory cannot be allocated or a previous error state is -// being overwritten. -#ifndef RCUTILS_REPORT_ERROR_HANDLING_ERRORS -#define RCUTILS_REPORT_ERROR_HANDLING_ERRORS 1 -#endif - -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD -#include -pthread_key_t __rcutils_error_state_key; -pthread_key_t __rcutils_error_string_key; -#else -RCUTILS_THREAD_LOCAL rcutils_error_state_t * __rcutils_error_state = NULL; -RCUTILS_THREAD_LOCAL char * __rcutils_error_string = NULL; -#endif - -static const char __error_format_string[] = "%s, at %s:%zu"; - -bool -__rcutils_error_is_set(rcutils_error_state_t * error_state); - -void -__rcutils_reset_error_string(char ** error_string_ptr, rcutils_allocator_t allocator); +// RCUTILS_REPORT_ERROR_HANDLING_ERRORS and RCUTILS_WARN_ON_TRUNCATION are set in the header below +#include "./error_handling_helpers.h" -void -__rcutils_reset_error(rcutils_error_state_t ** error_state_ptr_ptr); +// g_ is to global variable, as gtls_ is to global thread-local storage variable +RCUTILS_THREAD_LOCAL bool gtls_rcutils_thread_local_initialized = false; +RCUTILS_THREAD_LOCAL rcutils_error_state_t gtls_rcutils_error_state; +RCUTILS_THREAD_LOCAL bool gtls_rcutils_error_string_is_formatted = false; +RCUTILS_THREAD_LOCAL rcutils_error_string_t gtls_rcutils_error_string; +RCUTILS_THREAD_LOCAL bool gtls_rcutils_error_is_set = false; rcutils_ret_t -rcutils_error_state_copy(const rcutils_error_state_t * src, rcutils_error_state_t * dst) +rcutils_initialize_error_handling_thread_local_storage(rcutils_allocator_t allocator) { - dst->allocator = src->allocator; - dst->message = rcutils_strdup(src->message, dst->allocator); - if (NULL == dst->message) { - return RCUTILS_RET_BAD_ALLOC; + if (gtls_rcutils_thread_local_initialized) { + return RCUTILS_RET_OK; } - dst->file = rcutils_strdup(src->file, dst->allocator); - if (NULL == dst->file) { - return RCUTILS_RET_BAD_ALLOC; - } - dst->line_number = src->line_number; - return RCUTILS_RET_OK; -} - -void -rcutils_error_state_fini(rcutils_error_state_t * error_state) -{ - error_state->allocator.deallocate((char *)error_state->message, error_state->allocator.state); - error_state->allocator.deallocate((char *)error_state->file, error_state->allocator.state); -} -void -rcutils_set_error_state( - const char * error_string, - const char * file, - size_t line_number, - rcutils_allocator_t allocator) -{ + // check if the given allocator is valid if (!rcutils_allocator_is_valid(&allocator)) { #if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - // rcutils_allocator is invalid, logging to stderr instead RCUTILS_SAFE_FWRITE_TO_STDERR( "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) - "] provided allocator is invalid, error state not updated\n"); + "] rcutils_initialize_error_handling_thread_local_storage() given invalid allocator\n"); #endif - return; + return RCUTILS_RET_INVALID_ARGUMENT; } -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - rcutils_error_state_t * __rcutils_error_state = - (rcutils_error_state_t *)pthread_getspecific(__rcutils_error_state_key); - char * __rcutils_error_string = (char *)pthread_getspecific(__rcutils_error_string_key); -#endif - rcutils_error_state_t * old_error_state = __rcutils_error_state; -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - const char * old_error_string = rcutils_get_error_string_safe(); -#endif - __rcutils_error_state = (rcutils_error_state_t *)allocator.allocate( - sizeof(rcutils_error_state_t), allocator.state); - if (NULL == __rcutils_error_state) { -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - // rcutils_allocate failed, but fwrite might work? - RCUTILS_SAFE_FWRITE_TO_STDERR( - "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) - "] failed to allocate memory for the error state struct\n"); -#endif - return; - } - __rcutils_error_state->allocator = allocator; + // right now the allocator is not used for anything + // but other future implementations may need to use it + // e.g. pthread which could only provide thread-local pointers would need to + // allocate memory to which those pointers would point -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - pthread_setspecific(__rcutils_error_state_key, __rcutils_error_state); -#endif - size_t error_string_length = strlen(error_string); - // the memory must be one byte bigger to store the NULL character - __rcutils_error_state->message = - (char *)allocator.allocate(error_string_length + 1, allocator.state); - if (NULL == __rcutils_error_state->message) { -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - // malloc failed, but fwrite might work? - RCUTILS_SAFE_FWRITE_TO_STDERR( - "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) - "] failed to allocate memory for the error message in the error state struct\n"); -#endif - rcutils_reset_error(); // This will free any allocations done so far. - return; - } - // Cast the const away to set ->message initially. -#ifndef _WIN32 - snprintf((char *)__rcutils_error_state->message, error_string_length + 1, "%s", error_string); -#else - auto retcode = strcpy_s( - (char *)__rcutils_error_state->message, error_string_length + 1, error_string); - if (retcode) { -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - RCUTILS_SAFE_FWRITE_TO_STDERR( - "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) - "] failed to copy error message in the error state struct\n"); -#endif + // forcing the values back to their initial state should force the thread-local storage + // to initialize and do any required memory allocation + gtls_rcutils_thread_local_initialized = true; + rcutils_reset_error(); + RCUTILS_SET_ERROR_MSG("no error - initializing thread-local storage") + { // this scope is to prevent uncrustify from moving the (void) to the previous line + (void)rcutils_get_error_string(); } -#endif - __rcutils_error_state->file = file; - __rcutils_error_state->line_number = line_number; - if (__rcutils_error_is_set(old_error_state)) { -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - // Only warn of overwritting if the new error string is different from the old ones. - if (error_string != old_error_string && error_string != old_error_state->message) { - fprintf( - stderr, - "\n" - ">>> [rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) "] rcutils_set_error_state()\n" - "This error state is being overwritten:\n" - "\n" - " '%s'\n" - "\n" - "with this new error message:\n" - "\n" - " '%s, at %s:%zu'\n" - "\n" - "rcutils_reset_error() should be called after error handling to avoid this.\n" - "<<<\n", - old_error_string, - error_string, - file, - line_number); - } -#endif - __rcutils_reset_error(&old_error_state); - } - __rcutils_reset_error_string(&__rcutils_error_string, __rcutils_error_state->allocator); -} + rcutils_reset_error(); -const rcutils_error_state_t * -rcutils_get_error_state(void) -{ -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - return (rcutils_error_state_t *)pthread_getspecific(__rcutils_error_state_key); -#else - return __rcutils_error_state; -#endif + // at this point the thread-local allocator, error state, and error string are all initialized + return RCUTILS_RET_OK; } -static void -format_error_string(void) +static +bool +__same_string(const char * str1, const char * str2, size_t count) { -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - rcutils_error_state_t * __rcutils_error_state = - (rcutils_error_state_t *)pthread_getspecific(__rcutils_error_state_key); - char * __rcutils_error_string = (char *)pthread_getspecific(__rcutils_error_string_key); -#endif - if (!__rcutils_error_is_set(__rcutils_error_state)) { - return; - } - size_t bytes_it_would_have_written = snprintf( - NULL, 0, - __error_format_string, - __rcutils_error_state->message, - __rcutils_error_state->file, - __rcutils_error_state->line_number); - rcutils_allocator_t allocator = __rcutils_error_state->allocator; - __rcutils_error_string = - (char *)allocator.allocate(bytes_it_would_have_written + 1, allocator.state); -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - pthread_setspecific(__rcutils_error_string_key, __rcutils_error_string); -#endif - if (NULL == __rcutils_error_string) { -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - // rcutils_allocate failed, but fwrite might work? - RCUTILS_SAFE_FWRITE_TO_STDERR( - "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) - "] failed to allocate memory for the error string\n"); -#endif - return; - } - snprintf( - __rcutils_error_string, bytes_it_would_have_written + 1, - __error_format_string, - __rcutils_error_state->message, - __rcutils_error_state->file, - __rcutils_error_state->line_number); - // The Windows version of snprintf does not null terminate automatically in all cases. - __rcutils_error_string[bytes_it_would_have_written] = '\0'; + return str1 == str2 || 0 == strncmp(str1, str2, count); } -const char * -rcutils_get_error_string(void) +void +rcutils_set_error_state( + const char * error_string, + const char * file, + size_t line_number) { -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - char * __rcutils_error_string = (char *)pthread_getspecific(__rcutils_error_string_key); -#endif - if (NULL == __rcutils_error_string) { - format_error_string(); - } - return __rcutils_error_string; -} + rcutils_error_state_t error_state; -bool -__rcutils_error_is_set(rcutils_error_state_t * error_state) -{ - return error_state != NULL; + // The const is casted away to set the 'message' field of the error state (const for users only). + __rcutils_copy_string((char *)error_state.message, sizeof(error_state.message), error_string); + __rcutils_copy_string((char *)error_state.file, sizeof(error_state.file), file); + error_state.line_number = line_number; +#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS + // Only warn of overwritting if the new error is different from the old ones. + size_t characters_to_compare = strnlen(error_string, RCUTILS_ERROR_MESSAGE_MAX_LENGTH); + // assumption is that message length is <= max error string length + static_assert( + sizeof(gtls_rcutils_error_state.message) <= sizeof(gtls_rcutils_error_string.str), + "expected error state's max message length to be less than or equal to error string max"); + if ( + gtls_rcutils_error_is_set && + __same_string(error_string, gtls_rcutils_error_string.str, characters_to_compare) && + __same_string(error_string, gtls_rcutils_error_state.message, characters_to_compare)) + { + fprintf( + stderr, + "\n" + ">>> [rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) "] rcutils_set_error_state()\n" + "This error state is being overwritten:\n" + "\n" + " '%s'\n" + "\n" + "with this new error message:\n" + "\n" + " '%s, at %s:%zu'\n" + "\n" + "rcutils_reset_error() should be called after error handling to avoid this.\n" + "<<<\n", + gtls_rcutils_error_string.str, + error_string, + file, + line_number); + } +#endif + gtls_rcutils_error_state = error_state; + gtls_rcutils_error_string_is_formatted = false; + gtls_rcutils_error_string = (const rcutils_error_string_t) {0}; + gtls_rcutils_error_is_set = true; } bool rcutils_error_is_set(void) { -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - rcutils_error_state_t * __rcutils_error_state = - (rcutils_error_state_t *)pthread_getspecific(__rcutils_error_state_key); -#endif - return __rcutils_error_is_set(__rcutils_error_state); + return gtls_rcutils_error_is_set; } -const char * -rcutils_get_error_string_safe(void) +const rcutils_error_state_t * +rcutils_get_error_state(void) { - if (!rcutils_error_is_set()) { - return "error not set"; - } - return rcutils_get_error_string(); + return >ls_rcutils_error_state; } -void -__rcutils_reset_error_string(char ** error_string_ptr, rcutils_allocator_t allocator) +rcutils_error_string_t +rcutils_get_error_string(void) { - if (NULL == error_string_ptr) { - return; + if (!gtls_rcutils_error_is_set) { + return (rcutils_error_string_t) {"error not set"}; // NOLINT(readability/braces) } - - rcutils_allocator_t local_allocator = allocator; - if (!rcutils_allocator_is_valid(&allocator)) { -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - RCUTILS_SAFE_FWRITE_TO_STDERR( - "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) "]: " - "invalid allocator\n"); -#endif - local_allocator = rcutils_get_default_allocator(); - } - char * error_string = *error_string_ptr; - if (error_string != NULL) { - local_allocator.deallocate(error_string, local_allocator.state); - } - *error_string_ptr = NULL; -} - -void -__rcutils_reset_error(rcutils_error_state_t ** error_state_ptr_ptr) -{ - if (error_state_ptr_ptr != NULL) { - rcutils_error_state_t * error_state_ptr = *error_state_ptr_ptr; - if (error_state_ptr != NULL) { - rcutils_allocator_t allocator = error_state_ptr->allocator; - if (NULL == allocator.deallocate) { -#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS - RCUTILS_SAFE_FWRITE_TO_STDERR( - "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) "]: " - "invalid allocator, deallocate function pointer is null\n"); -#endif - allocator = rcutils_get_default_allocator(); - } - if (error_state_ptr->message != NULL) { - // Cast const away to delete previously allocated memory. - allocator.deallocate((char *)error_state_ptr->message, allocator.state); - } - allocator.deallocate(error_state_ptr, allocator.state); - } - *error_state_ptr_ptr = NULL; + if (!gtls_rcutils_error_string_is_formatted) { + __rcutils_format_error_string(>ls_rcutils_error_string, >ls_rcutils_error_state); } + return gtls_rcutils_error_string; } void rcutils_reset_error(void) { -#ifdef RCUTILS_THREAD_LOCAL_PTHREAD - rcutils_error_state_t * __rcutils_error_state = - (rcutils_error_state_t *)pthread_getspecific(__rcutils_error_state_key); - char * __rcutils_error_string = (char *)pthread_getspecific(__rcutils_error_string_key); -#endif - if (__rcutils_error_state != NULL) { - __rcutils_reset_error_string(&__rcutils_error_string, __rcutils_error_state->allocator); - } - __rcutils_reset_error(&__rcutils_error_state); + gtls_rcutils_error_state = (const rcutils_error_state_t) { + .message = {0}, .file = {0}, .line_number = 0 + }; // NOLINT(readability/braces) + gtls_rcutils_error_string_is_formatted = false; + gtls_rcutils_error_string = (const rcutils_error_string_t) {0}; + gtls_rcutils_error_is_set = false; } diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h new file mode 100644 index 00000000..63f0fdfb --- /dev/null +++ b/src/error_handling_helpers.h @@ -0,0 +1,160 @@ +// Copyright 2014 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Note: migrated from rmw/error_handling.h in 2017-04 + +#ifndef ERROR_HANDLING_HELPERS_H_ +#define ERROR_HANDLING_HELPERS_H_ + +// When this define evaluates to true (default), then messages will printed to +// stderr when an error is encountered while setting the error state. +// For example, when memory cannot be allocated or a previous error state is +// being overwritten. +#ifndef RCUTILS_REPORT_ERROR_HANDLING_ERRORS +#define RCUTILS_REPORT_ERROR_HANDLING_ERRORS 1 +#endif + +// When this define evaluates to true (default), then a warning will be written +// to stderr using RCUTILS_SAFE_FWRITE_TO_STDERR. +#ifndef RCUTILS_WARN_ON_TRUNCATION +#define RCUTILS_WARN_ON_TRUNCATION 1 +#endif + +#define __STDC_WANT_LIB_EXT1__ 1 // indicate we would like memmove_s if available +#include +#include +#include + +#include + +#ifdef __cplusplus +extern "C" +{ +#endif + +size_t +__rcutils_copy_string(char * dst, size_t dst_size, const char * src) +{ + // doesn't matter how long src actually is if it is longer than dst, so limit to dst + 1 + size_t src_length = strnlen(src, dst_size + 1); + size_t size_to_copy = src_length; + // the destination must be one byte bigger to store the NULL terminating character + if (src_length >= dst_size) { + size_to_copy = dst_size - 1; +#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS && RCUTILS_WARN_ON_TRUNCATION + // truncation will be required + RCUTILS_SAFE_FWRITE_TO_STDERR( + "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) + "] an error string (message, file name, or formatted message) will be truncated\n"); +#endif + } + +#ifdef __STDC_LIB_EXT1__ + errno_t ret = memmove_s( + dst, dst_size, + src, size_to_copy); // this will always ensure truncation + if (0 != ret) { +# if RCUTILS_REPORT_ERROR_HANDLING_ERRORS + RCUTILS_SAFE_FWRITE_TO_STDERR( + "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) + "] memmove_s failed, the error string may be malformed\n"); +# endif + } +#else + (void)memmove(dst, src, size_to_copy); +#endif + dst[size_to_copy] = '\0'; + return size_to_copy; +} + +void +__rcutils_reverse_str(char * string_in, size_t string_len) +{ + size_t i = 0; + size_t j = string_len - 1; + for (; i < j; i++, j--) { + char c = string_in[i]; + string_in[i] = string_in[j]; + string_in[j] = c; + } +} + +// sizeof(buffer) must be greater than or equal to 21. +void +__rcutils_convert_uint64_t_into_c_str(uint64_t number, char * buffer) +{ + size_t i = 0; + + // if number is 0, short circuit + if (number == 0) { + buffer[0] = '0'; + buffer[1] = '\0'; + return; + } + + // add the modulo 10 to thes string and then integer divide by 10 until 0 + while (number != 0) { + buffer[i++] = number % 10 + '0'; + number = number / 10; + } + + // null terminate + buffer[i] = '\0'; + + // reverse the string in place + __rcutils_reverse_str(buffer, strnlen(buffer, 21)); +} + +void +__rcutils_format_error_string( + rcutils_error_string_t * error_string, + const rcutils_error_state_t * error_state) +{ + static const char format_1[] = ", at "; + static const char format_2[] = ":"; + static char line_number_buffer[21]; + static_assert( + sizeof(error_string->str) == ( + sizeof(error_state->message) + + sizeof(format_1) - 1 /* minus the null-term */ + + sizeof(error_state->file) + + sizeof(format_2) - 1 /* minus the null-term */ + + sizeof(line_number_buffer) - 1 /* minus the null-term */ + + 1 // null terminator + ), "math error in static string formatting"); + char * offset = (char *)error_string->str; + size_t bytes_left = sizeof(error_string->str); + size_t written = __rcutils_copy_string(offset, bytes_left, error_state->message); + offset += written; + bytes_left -= written; + written = __rcutils_copy_string(offset, bytes_left, format_1); + offset += written; + bytes_left -= written; + written = __rcutils_copy_string(offset, bytes_left, error_state->file); + offset += written; + bytes_left -= written; + written = __rcutils_copy_string(offset, bytes_left, format_2); + offset += written; + bytes_left -= written; + __rcutils_convert_uint64_t_into_c_str(error_state->line_number, line_number_buffer); + written = __rcutils_copy_string(offset, bytes_left, line_number_buffer); + offset += written; + offset[0] = '\0'; +} + +#ifdef __cplusplus +} +#endif + +#endif // ERROR_HANDLING_HELPERS_H_ diff --git a/test/test_error_handling.cpp b/test/test_error_handling.cpp index 17d7d3fa..232abfa3 100644 --- a/test/test_error_handling.cpp +++ b/test/test_error_handling.cpp @@ -16,6 +16,8 @@ #include "gmock/gmock.h" +#include "osrf_testing_tools_cpp/memory_tools/gtest_quickstart.hpp" + #include "rcutils/error_handling.h" int @@ -36,52 +38,164 @@ count_substrings(const std::string & str, const std::string & substr) } TEST(test_error_handling, nominal) { - rcutils_reset_error(); + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest(true); + + // Note that without the thread-local initialization function below, you might get errors like: + /** +[ RUN ] test_error_handling.nominal +/.../osrf_testing_tools_cpp/memory_tools/gtest_quickstart.hpp:178: Failure +Failed +unexpected call to malloc +test_error_handling(36072,0x7fffb391b380) malloc: malloc (not expected) 2040 -> 0x7f9b6a002400 +Stack trace (most recent call last): +#7 Object "libdyld.dylib", at 0x7fff7ae09969, + in tlv_allocate_and_initialize_for_key + 322 +#6 Object "libmemory_tools_interpose.dylib", + at 0x105cee37b, + in apple_replacement_malloc + 27 +#5 Object "libmemory_tools_interpose.dylib", + at 0x105cee43e, + in unix_replacement_malloc + 174 +#4 Object "libmemory_tools.dylib", + at 0x105d08732, + in osrf_testing_tools_cpp::memory_tools::custom_malloc_with_original( + unsigned long, void* (*)(unsigned long), char const*, bool) + 50 +#3 Object "libmemory_tools.dylib", + at 0x105d0896b, + in osrf_testing_tools_cpp::memory_tools::custom_malloc_with_original_except( + unsigned long, void* (*)(unsigned long), char const*, bool) + 395 +#2 Object "libmemory_tools.dylib", + at 0x105d094a1, + in void osrf_testing_tools_cpp::memory_tools::print_backtrace<64>(__sFILE*) + 49 +#1 Object "libmemory_tools.dylib", + at 0x105d097c5, + in backward::StackTraceImpl::load_here(unsigned long) + 101 +#0 Object "libmemory_tools.dylib", + at 0x105d09f26, + in unsigned long backward::details::unwind::callback>( + backward::StackTraceImpl::callback, unsigned long) + 38 +[ FAILED ] test_error_handling.nominal (7 ms) + */ + // Note that the error is in `tlv_allocate_and_initialize_for_key`, which is the thread-local + // storage setup function, at least for clang on macOS. + rcutils_ret_t ret = + rcutils_initialize_error_handling_thread_local_storage(rcutils_get_default_allocator()); + ASSERT_EQ(ret, RCUTILS_RET_OK); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message, rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG(test_message); using ::testing::StartsWith; - ASSERT_THAT(rcutils_get_error_string_safe(), StartsWith(test_message)); - rcutils_reset_error(); + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_THAT(error_string.str, StartsWith(test_message)); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); } TEST(test_error_handling, reset) { - rcutils_reset_error(); + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + rcutils_ret_t ret = + rcutils_initialize_error_handling_thread_local_storage(rcutils_get_default_allocator()); + ASSERT_EQ(ret, RCUTILS_RET_OK); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); { const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message, rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG(test_message); using ::testing::StartsWith; - ASSERT_THAT(rcutils_get_error_string_safe(), StartsWith(test_message)); + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_THAT(error_string.str, StartsWith(test_message)); } rcutils_reset_error(); { const char * test_message = "different message"; - RCUTILS_SET_ERROR_MSG(test_message, rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG(test_message); using ::testing::StartsWith; - ASSERT_THAT(rcutils_get_error_string_safe(), StartsWith(test_message)); + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_THAT(error_string.str, StartsWith(test_message)); } - rcutils_reset_error(); - ASSERT_EQ(nullptr, rcutils_get_error_string()); - ASSERT_NE(nullptr, rcutils_get_error_string_safe()); - ASSERT_STREQ("error not set", rcutils_get_error_string_safe()); - rcutils_reset_error(); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_NE(nullptr, error_string.str); + } + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_STREQ("error not set", error_string.str); + } + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); } TEST(test_error_handling, empty) { - rcutils_reset_error(); - ASSERT_EQ(nullptr, rcutils_get_error_string()); - ASSERT_NE(nullptr, rcutils_get_error_string_safe()); - ASSERT_STREQ("error not set", rcutils_get_error_string_safe()); - rcutils_reset_error(); + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + rcutils_ret_t ret = + rcutils_initialize_error_handling_thread_local_storage(rcutils_get_default_allocator()); + ASSERT_EQ(ret, RCUTILS_RET_OK); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_NE(nullptr, error_string.str); + } + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_STREQ("error not set", error_string.str); + } + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); } TEST(test_error_handling, recursive) { - rcutils_reset_error(); + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + rcutils_ret_t ret = + rcutils_initialize_error_handling_thread_local_storage(rcutils_get_default_allocator()); + ASSERT_EQ(ret, RCUTILS_RET_OK); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message, rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG(test_message); using ::testing::HasSubstr; - ASSERT_THAT(rcutils_get_error_string_safe(), HasSubstr(", at")); - RCUTILS_SET_ERROR_MSG(rcutils_get_error_string_safe(), rcutils_get_default_allocator()); - std::string err_msg(rcutils_get_error_string_safe()); + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_THAT(error_string.str, HasSubstr(", at")); + } + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(rcutils_get_error_string().str); + }); + std::string err_msg; + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + err_msg = error_string.str; + } int count = count_substrings(err_msg, ", at"); EXPECT_EQ(2, count) << "Expected ', at' in the error string twice but got it '" << count << "': " << err_msg; @@ -89,22 +203,33 @@ TEST(test_error_handling, recursive) { } TEST(test_error_handling, copy) { - rcutils_reset_error(); + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + rcutils_ret_t ret = + rcutils_initialize_error_handling_thread_local_storage(rcutils_get_default_allocator()); + ASSERT_EQ(ret, RCUTILS_RET_OK); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message, rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG(test_message); using ::testing::HasSubstr; - ASSERT_THAT(rcutils_get_error_string_safe(), HasSubstr(", at")); - rcutils_error_state_t copy; - rcutils_ret_t ret = rcutils_error_state_copy(rcutils_get_error_state(), ©); - ASSERT_EQ(RCUTILS_RET_OK, ret); + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_THAT(error_string.str, HasSubstr(", at")); + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_state_t copy = *rcutils_get_error_state(); + EXPECT_NO_MEMORY_OPERATIONS_END(); // ensure the copy is correct ASSERT_STREQ(test_message, copy.message); ASSERT_STREQ(__FILE__, copy.file); // ensure the original is still right too - auto error_state = rcutils_get_error_state(); + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + const rcutils_error_state_t * error_state = rcutils_get_error_state(); + EXPECT_NO_MEMORY_OPERATIONS_END(); ASSERT_NE(nullptr, error_state); ASSERT_STREQ(test_message, error_state->message); - // cleanup - rcutils_error_state_fini(©); - rcutils_reset_error(); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); } diff --git a/test/test_error_handling_helpers.cpp b/test/test_error_handling_helpers.cpp new file mode 100644 index 00000000..ba0f8f65 --- /dev/null +++ b/test/test_error_handling_helpers.cpp @@ -0,0 +1,143 @@ +// Copyright 2016 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "gmock/gmock.h" + +#include "osrf_testing_tools_cpp/memory_tools/gtest_quickstart.hpp" + +#include "../src/error_handling_helpers.h" + +TEST(test_error_handling, copy_string) { + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + char buffer[10]; + memset(buffer, '?', sizeof(buffer)); + size_t written; + + // normal truncation + EXPECT_NO_MEMORY_OPERATIONS({ + written = __rcutils_copy_string(buffer, 3, "0123456789"); + }); + EXPECT_EQ(written, 2u); + EXPECT_EQ(strnlen(buffer, sizeof(buffer)), 2u); + EXPECT_STREQ(buffer, "01"); + + // normal truncation, 1 short of buffer length + EXPECT_NO_MEMORY_OPERATIONS({ + written = __rcutils_copy_string(buffer, 9, "0123456789"); + }); + EXPECT_EQ(written, 8u); + EXPECT_EQ(strnlen(buffer, sizeof(buffer)), 8u); + EXPECT_STREQ(buffer, "01234567"); + + // input smaller than buffer, 1 short of buffer length + EXPECT_NO_MEMORY_OPERATIONS({ + written = __rcutils_copy_string(buffer, 9, ""); + }); + EXPECT_EQ(written, 0u); + EXPECT_EQ(strnlen(buffer, sizeof(buffer)), 0u); + EXPECT_STREQ(buffer, ""); + + // copy where src and dst overlap (testing use of memmove vs memcpy) + EXPECT_NO_MEMORY_OPERATIONS({ + written = __rcutils_copy_string(buffer, sizeof(buffer), "1234567890"); + }); + EXPECT_EQ(written, 9u); + EXPECT_STREQ(buffer, "123456789"); + EXPECT_NO_MEMORY_OPERATIONS({ + written = __rcutils_copy_string(buffer, sizeof(buffer), buffer + 3); + }); + EXPECT_EQ(written, 6u); + EXPECT_EQ(strnlen(buffer, sizeof(buffer)), (sizeof(buffer) - 1) - 3); + EXPECT_STREQ(buffer, "456789"); +} + +TEST(test_error_handling, reverse_str) { + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + { + char buffer[] = "even"; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_reverse_str(buffer, strnlen(buffer, sizeof(buffer))); + }); + EXPECT_STREQ(buffer, "neve"); + } + { + char buffer[] = "reverseme"; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_reverse_str(buffer, strnlen(buffer, sizeof(buffer))); + }); + EXPECT_STREQ(buffer, "emesrever"); + } + { + char buffer[] = "a"; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_reverse_str(buffer, strnlen(buffer, sizeof(buffer))); + }); + EXPECT_STREQ(buffer, "a"); + } + { + char buffer[] = "reverseme"; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_reverse_str(buffer, 3); + }); + EXPECT_STREQ(buffer, "vererseme"); + } +} + +TEST(test_error_handling, convert_uint64_t_into_c_str) { + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + { + uint64_t number = UINT64_MAX; + char buffer[21]; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_convert_uint64_t_into_c_str(number, buffer); + }); + EXPECT_STREQ(buffer, "18446744073709551615"); + } + { + uint64_t number = 0; + char buffer[21]; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_convert_uint64_t_into_c_str(number, buffer); + }); + EXPECT_STREQ(buffer, "0"); + } + { + uint64_t number = 42; + char buffer[21]; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_convert_uint64_t_into_c_str(number, buffer); + }); + EXPECT_STREQ(buffer, "42"); + } + { + uint64_t number = INT64_MAX; + char buffer[21]; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_convert_uint64_t_into_c_str(number, buffer); + }); + EXPECT_STREQ(buffer, "9223372036854775807"); + } +} + +TEST(test_error_handling, format_error_string) { + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + rcutils_error_string_t error_string {""}; + rcutils_error_state_t error_state {"test error message", "/path/to/source", 42}; + + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_format_error_string(&error_string, &error_state); + }); +} From 330cf30bdfccc20732165f9d02fc6386d2460e5f Mon Sep 17 00:00:00 2001 From: William Woodall Date: Mon, 29 Oct 2018 10:53:20 -0400 Subject: [PATCH 02/20] use new rcutils error handling API Signed-off-by: William Woodall --- include/rcutils/allocator.h | 2 +- include/rcutils/logging.h | 2 +- src/char_array.c | 24 ++--- src/logging.c | 36 +++---- src/split.c | 8 +- src/string_array.c | 10 +- src/string_map.c | 103 +++++++----------- src/time.c | 14 ++- src/time_unix.c | 10 +- src/time_win32.c | 6 +- test/test_logging_long_messages.cpp | 2 +- test/test_split.cpp | 2 +- test/test_string_map.cpp | 162 ++++++++++++++-------------- test/test_time.cpp | 36 +++---- 14 files changed, 184 insertions(+), 233 deletions(-) diff --git a/include/rcutils/allocator.h b/include/rcutils/allocator.h index a7d8008d..d92b9f2a 100644 --- a/include/rcutils/allocator.h +++ b/include/rcutils/allocator.h @@ -122,7 +122,7 @@ rcutils_allocator_is_valid(const rcutils_allocator_t * allocator); #define RCUTILS_CHECK_ALLOCATOR_WITH_MSG(allocator, msg, fail_statement) \ if (!rcutils_allocator_is_valid(allocator)) { \ - RCUTILS_SET_ERROR_MSG(msg, rcutils_get_default_allocator()) \ + RCUTILS_SET_ERROR_MSG(msg) \ fail_statement; \ } diff --git a/include/rcutils/logging.h b/include/rcutils/logging.h index f39e7488..61fe07e6 100644 --- a/include/rcutils/logging.h +++ b/include/rcutils/logging.h @@ -506,7 +506,7 @@ void rcutils_logging_console_output_handler( RCUTILS_SAFE_FWRITE_TO_STDERR( \ "[rcutils|" __FILE__ ":" RCUTILS_STRINGIFY(__LINE__) \ "] error initializing logging: "); \ - RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string_safe()); \ + RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string().str); \ RCUTILS_SAFE_FWRITE_TO_STDERR("\n"); \ rcutils_reset_error(); \ } \ diff --git a/src/char_array.c b/src/char_array.c index 9b3ba7bd..f4d5e9ff 100644 --- a/src/char_array.c +++ b/src/char_array.c @@ -33,15 +33,13 @@ rcutils_char_array_init( size_t buffer_capacity, const rcutils_allocator_t * allocator) { - rcutils_allocator_t error_msg_allocator = rcutils_get_default_allocator(); RCUTILS_CHECK_FOR_NULL_WITH_MSG( char_array, "char array pointer is null", - return RCUTILS_RET_ERROR, - error_msg_allocator); + return RCUTILS_RET_ERROR); if (!rcutils_allocator_is_valid(allocator)) { - RCUTILS_SET_ERROR_MSG("char array has no valid allocator", error_msg_allocator); + RCUTILS_SET_ERROR_MSG("char array has no valid allocator"); return RCUTILS_RET_ERROR; } @@ -55,8 +53,7 @@ rcutils_char_array_init( RCUTILS_CHECK_FOR_NULL_WITH_MSG( char_array->buffer, "failed to allocate memory for char array", - return RCUTILS_RET_BAD_ALLOC, - *allocator); + return RCUTILS_RET_BAD_ALLOC); } return RCUTILS_RET_OK; @@ -65,16 +62,14 @@ rcutils_char_array_init( rcutils_ret_t rcutils_char_array_fini(rcutils_char_array_t * char_array) { - rcutils_allocator_t error_msg_allocator = rcutils_get_default_allocator(); RCUTILS_CHECK_FOR_NULL_WITH_MSG( char_array, "char array pointer is null", - return RCUTILS_RET_ERROR, - error_msg_allocator); + return RCUTILS_RET_ERROR); rcutils_allocator_t * allocator = &char_array->allocator; if (!rcutils_allocator_is_valid(allocator)) { - RCUTILS_SET_ERROR_MSG("char array has no valid allocator", error_msg_allocator); + RCUTILS_SET_ERROR_MSG("char array has no valid allocator"); return RCUTILS_RET_ERROR; } @@ -89,16 +84,14 @@ rcutils_char_array_fini(rcutils_char_array_t * char_array) rcutils_ret_t rcutils_char_array_resize(rcutils_char_array_t * char_array, size_t new_size) { - rcutils_allocator_t error_msg_allocator = rcutils_get_default_allocator(); RCUTILS_CHECK_FOR_NULL_WITH_MSG( char_array, "char array pointer is null", - return RCUTILS_RET_ERROR, - error_msg_allocator); + return RCUTILS_RET_ERROR); rcutils_allocator_t * allocator = &char_array->allocator; if (!rcutils_allocator_is_valid(allocator)) { - RCUTILS_SET_ERROR_MSG("char array has no valid allocator", error_msg_allocator); + RCUTILS_SET_ERROR_MSG("char array has no valid allocator"); return RCUTILS_RET_ERROR; } @@ -111,8 +104,7 @@ rcutils_char_array_resize(rcutils_char_array_t * char_array, size_t new_size) RCUTILS_CHECK_FOR_NULL_WITH_MSG( char_array->buffer, "failed to reallocate memory for char array", - return RCUTILS_RET_BAD_ALLOC, - *allocator); + return RCUTILS_RET_BAD_ALLOC); char_array->buffer_capacity = new_size; if (new_size < char_array->buffer_length) { diff --git a/src/logging.c b/src/logging.c index fbaf3faf..7a79bb04 100644 --- a/src/logging.c +++ b/src/logging.c @@ -92,8 +92,7 @@ rcutils_ret_t rcutils_logging_initialize_with_allocator(rcutils_allocator_t allo rcutils_ret_t ret = RCUTILS_RET_OK; if (!g_rcutils_logging_initialized) { if (!rcutils_allocator_is_valid(&allocator)) { - RCUTILS_SET_ERROR_MSG( - "Provided allocator is invalid.", rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG("Provided allocator is invalid."); return RCUTILS_RET_INVALID_ARGUMENT; } g_rcutils_logging_allocator = allocator; @@ -114,7 +113,6 @@ rcutils_ret_t rcutils_logging_initialize_with_allocator(rcutils_allocator_t allo } else { if (NULL != ret_str) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - g_rcutils_logging_allocator, "Failed to get output format from env. variable [%s]. Using default output format.", ret_str); ret = RCUTILS_RET_INVALID_ARGUMENT; @@ -129,9 +127,8 @@ rcutils_ret_t rcutils_logging_initialize_with_allocator(rcutils_allocator_t allo if (string_map_ret != RCUTILS_RET_OK) { // If an error message was set it will have been overwritten by rcutils_string_map_init. RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - g_rcutils_logging_allocator, "Failed to initialize map for logger severities [%s]. Severities will not be configurable.", - rcutils_get_error_string_safe()); + rcutils_get_error_string().str); g_rcutils_logging_severities_map_valid = false; ret = RCUTILS_RET_STRING_MAP_INVALID; } else { @@ -153,9 +150,8 @@ rcutils_ret_t rcutils_logging_shutdown(void) rcutils_ret_t string_map_ret = rcutils_string_map_fini(&g_rcutils_logging_severities_map); if (string_map_ret != RCUTILS_RET_OK) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - g_rcutils_logging_allocator, "Failed to finalize map for logger severities: %s", - rcutils_get_error_string_safe()); + rcutils_get_error_string().str); ret = RCUTILS_RET_LOGGING_SEVERITY_MAP_INVALID; } g_rcutils_logging_severities_map_valid = false; @@ -170,16 +166,15 @@ rcutils_logging_severity_level_from_string( { RCUTILS_CHECK_ALLOCATOR_WITH_MSG( &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_ARGUMENT_FOR_NULL(severity_string, RCUTILS_RET_INVALID_ARGUMENT, allocator); - RCUTILS_CHECK_ARGUMENT_FOR_NULL(severity, RCUTILS_RET_INVALID_ARGUMENT, allocator); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(severity_string, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(severity, RCUTILS_RET_INVALID_ARGUMENT); rcutils_ret_t ret = RCUTILS_RET_LOGGING_SEVERITY_STRING_INVALID; // Convert the input string to upper case (for case insensitivity). char * severity_string_upper = rcutils_strdup(severity_string, allocator); if (NULL == severity_string_upper) { - RCUTILS_SET_ERROR_MSG( - "failed to allocate memory for uppercase string", rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG("failed to allocate memory for uppercase string"); return RCUTILS_RET_BAD_ALLOC; } for (int i = 0; severity_string_upper[i]; ++i) { @@ -316,8 +311,7 @@ rcutils_ret_t rcutils_logging_set_logger_level(const char * name, int level) { RCUTILS_LOGGING_AUTOINIT if (NULL == name) { - RCUTILS_SET_ERROR_MSG( - "Invalid logger name", g_rcutils_logging_allocator); + RCUTILS_SET_ERROR_MSG("Invalid logger name"); return RCUTILS_RET_INVALID_ARGUMENT; } if (strlen(name) == 0) { @@ -325,8 +319,7 @@ rcutils_ret_t rcutils_logging_set_logger_level(const char * name, int level) return RCUTILS_RET_OK; } if (!g_rcutils_logging_severities_map_valid) { - RCUTILS_SET_ERROR_MSG( - "Logger severity level map is invalid", g_rcutils_logging_allocator); + RCUTILS_SET_ERROR_MSG("Logger severity level map is invalid"); return RCUTILS_RET_LOGGING_SEVERITY_MAP_INVALID; } @@ -336,23 +329,20 @@ rcutils_ret_t rcutils_logging_set_logger_level(const char * name, int level) level >= (int)(sizeof(g_rcutils_log_severity_names) / sizeof(g_rcutils_log_severity_names[0]))) { - RCUTILS_SET_ERROR_MSG( - "Invalid severity level specified for logger", g_rcutils_logging_allocator); + RCUTILS_SET_ERROR_MSG("Invalid severity level specified for logger"); return RCUTILS_RET_INVALID_ARGUMENT; } const char * severity_string = g_rcutils_log_severity_names[level]; if (NULL == severity_string) { - RCUTILS_SET_ERROR_MSG( - "Unable to determine severity_string for severity", g_rcutils_logging_allocator); + RCUTILS_SET_ERROR_MSG("Unable to determine severity_string for severity"); return RCUTILS_RET_INVALID_ARGUMENT; } rcutils_ret_t string_map_ret = rcutils_string_map_set( &g_rcutils_logging_severities_map, name, severity_string); if (string_map_ret != RCUTILS_RET_OK) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING( - g_rcutils_logging_allocator, "Error setting severity level for logger named '%s': %s", - name, rcutils_get_error_string_safe()); + name, rcutils_get_error_string().str); return RCUTILS_RET_ERROR; } return RCUTILS_RET_OK; @@ -603,7 +593,7 @@ void rcutils_logging_console_output_handler( ×tamp, numeric_storage, sizeof(numeric_storage)); if (ret != RCUTILS_RET_OK) { - RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string_safe()); + RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string().str); rcutils_reset_error(); RCUTILS_SAFE_FWRITE_TO_STDERR("\n"); goto cleanup; @@ -614,7 +604,7 @@ void rcutils_logging_console_output_handler( ×tamp, numeric_storage, sizeof(numeric_storage)); if (ret != RCUTILS_RET_OK) { - RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string_safe()); + RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string().str); rcutils_reset_error(); RCUTILS_SAFE_FWRITE_TO_STDERR("\n"); goto cleanup; diff --git a/src/split.c b/src/split.c index 1bc55820..6addd42d 100644 --- a/src/split.c +++ b/src/split.c @@ -35,7 +35,7 @@ rcutils_split( rcutils_string_array_t * string_array) { if (NULL == string_array) { - RCUTILS_SET_ERROR_MSG("string_array is null", allocator) + RCUTILS_SET_ERROR_MSG("string_array is null") return RCUTILS_RET_INVALID_ARGUMENT; } if (NULL == str || strlen(str) == 0) { @@ -113,12 +113,12 @@ rcutils_split( fail: if (rcutils_string_array_fini(string_array) != RCUTILS_RET_OK) { RCUTILS_SAFE_FWRITE_TO_STDERR("failed to finalize string array during error handling: "); - RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string_safe()); + RCUTILS_SAFE_FWRITE_TO_STDERR(rcutils_get_error_string().str); RCUTILS_SAFE_FWRITE_TO_STDERR("\n"); rcutils_reset_error(); } - RCUTILS_SET_ERROR_MSG("unable to allocate memory for string array data", allocator); + RCUTILS_SET_ERROR_MSG("unable to allocate memory for string array data"); return RCUTILS_RET_ERROR; } @@ -208,7 +208,7 @@ rcutils_split_last( fail: if (rcutils_string_array_fini(string_array) != RCUTILS_RET_OK) { RCUTILS_LOG_ERROR( - "failed to clean up on error (leaking memory): '%s'", rcutils_get_error_string_safe()); + "failed to clean up on error (leaking memory): '%s'", rcutils_get_error_string().str); } return result_error; } diff --git a/src/string_array.c b/src/string_array.c index 82497cc7..0085ab27 100644 --- a/src/string_array.c +++ b/src/string_array.c @@ -42,17 +42,17 @@ rcutils_string_array_init( const rcutils_allocator_t * allocator) { if (NULL == allocator) { - RCUTILS_SET_ERROR_MSG("allocator is null", rcutils_get_default_allocator()) + RCUTILS_SET_ERROR_MSG("allocator is null") return RCUTILS_RET_INVALID_ARGUMENT; } if (NULL == string_array) { - RCUTILS_SET_ERROR_MSG("string_array is null", *allocator) + RCUTILS_SET_ERROR_MSG("string_array is null") return RCUTILS_RET_INVALID_ARGUMENT; } string_array->size = size; string_array->data = allocator->zero_allocate(size, sizeof(char *), allocator->state); if (NULL == string_array->data) { - RCUTILS_SET_ERROR_MSG("failed to allocator string array", *allocator) + RCUTILS_SET_ERROR_MSG("failed to allocator string array") return RCUTILS_RET_BAD_ALLOC; } string_array->allocator = *allocator; @@ -63,7 +63,7 @@ rcutils_ret_t rcutils_string_array_fini(rcutils_string_array_t * string_array) { if (NULL == string_array) { - RCUTILS_SET_ERROR_MSG("string_array is null", rcutils_get_default_allocator()) + RCUTILS_SET_ERROR_MSG("string_array is null") return RCUTILS_RET_INVALID_ARGUMENT; } @@ -73,7 +73,7 @@ rcutils_string_array_fini(rcutils_string_array_t * string_array) rcutils_allocator_t * allocator = &string_array->allocator; if (!rcutils_allocator_is_valid(allocator)) { - RCUTILS_SET_ERROR_MSG("allocator is invalid", rcutils_get_default_allocator()) + RCUTILS_SET_ERROR_MSG("allocator is invalid") return RCUTILS_RET_INVALID_ARGUMENT; } size_t i; diff --git a/src/string_map.c b/src/string_map.c index 34716f0a..8ebc560e 100644 --- a/src/string_map.c +++ b/src/string_map.c @@ -52,19 +52,16 @@ rcutils_string_map_init( size_t initial_capacity, rcutils_allocator_t allocator) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT, allocator) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) if (string_map->impl != NULL) { - RCUTILS_SET_ERROR_MSG("string_map already initialized", allocator) + RCUTILS_SET_ERROR_MSG("string_map already initialized") return RCUTILS_RET_STRING_MAP_ALREADY_INIT; } RCUTILS_CHECK_ALLOCATOR_WITH_MSG( &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT) string_map->impl = allocator.allocate(sizeof(rcutils_string_map_impl_t), allocator.state); if (NULL == string_map->impl) { - RCUTILS_SET_ERROR_MSG( - "failed to allocate memory for string map impl struct", - // try default allocator, assuming given allocator is not able to allocate memory - rcutils_get_default_allocator()) + RCUTILS_SET_ERROR_MSG("failed to allocate memory for string map impl struct") return RCUTILS_RET_BAD_ALLOC; } string_map->impl->keys = NULL; @@ -85,8 +82,7 @@ rcutils_string_map_init( rcutils_ret_t rcutils_string_map_fini(rcutils_string_map_t * string_map) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) if (NULL == string_map->impl) { return RCUTILS_RET_OK; } @@ -111,13 +107,10 @@ rcutils_string_map_fini(rcutils_string_map_t * string_map) rcutils_ret_t rcutils_string_map_get_capacity(const rcutils_string_map_t * string_map, size_t * capacity) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - capacity, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(capacity, RCUTILS_RET_INVALID_ARGUMENT) // *INDENT-OFF* (prevent uncrustify getting this wrong) *capacity = string_map->impl->capacity; // *INDENT-ON* @@ -127,25 +120,22 @@ rcutils_string_map_get_capacity(const rcutils_string_map_t * string_map, size_t rcutils_ret_t rcutils_string_map_get_size(const rcutils_string_map_t * string_map, size_t * size) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - size, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) * - size = string_map->impl->size; + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(size, RCUTILS_RET_INVALID_ARGUMENT) + { // otherwise uncrustify moves the * to the end of the previous line... + *size = string_map->impl->size; + } return RCUTILS_RET_OK; } rcutils_ret_t rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) rcutils_allocator_t allocator = string_map->impl->allocator; // short circuit, if requested capacity is less than the size of the map if (capacity < string_map->impl->size) { @@ -169,7 +159,7 @@ rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) // ensure that reallocate won't overflow capacity if (capacity > (SIZE_MAX / sizeof(char *))) { - RCUTILS_SET_ERROR_MSG("requested capacity for string_map too large", allocator) + RCUTILS_SET_ERROR_MSG("requested capacity for string_map too large") return RCUTILS_RET_BAD_ALLOC; } @@ -177,7 +167,7 @@ rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) char ** new_keys = allocator.reallocate(string_map->impl->keys, capacity * sizeof(char *), allocator.state); if (NULL == new_keys) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map keys", allocator) + RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map keys") return RCUTILS_RET_BAD_ALLOC; } string_map->impl->keys = new_keys; @@ -186,7 +176,7 @@ rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) char ** new_values = allocator.reallocate(string_map->impl->values, capacity * sizeof(char *), allocator.state); if (NULL == new_values) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map values", allocator) + RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map values") return RCUTILS_RET_BAD_ALLOC; } string_map->impl->values = new_values; @@ -219,11 +209,9 @@ __remove_key_and_value_at_index(rcutils_string_map_impl_t * string_map_impl, siz rcutils_ret_t rcutils_string_map_clear(rcutils_string_map_t * string_map) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) size_t i = 0; for (; i < string_map->impl->capacity; ++i) { if (string_map->impl->keys[i] != NULL) { @@ -236,15 +224,11 @@ rcutils_string_map_clear(rcutils_string_map_t * string_map) rcutils_ret_t rcutils_string_map_set(rcutils_string_map_t * string_map, const char * key, const char * value) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - key, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - value, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(key, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT) rcutils_ret_t ret = rcutils_string_map_set_no_resize(string_map, key, value); // if it fails due to not enough space, resize and try again if (ret == RCUTILS_RET_NOT_ENOUGH_SPACE) { @@ -292,15 +276,11 @@ rcutils_string_map_set_no_resize( const char * key, const char * value) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - key, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - value, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(key, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT) rcutils_allocator_t allocator = string_map->impl->allocator; size_t key_index; bool should_free_key_on_error = false; @@ -319,7 +299,7 @@ rcutils_string_map_set_no_resize( assert(key_index < string_map->impl->capacity); // defensive, this should not happen string_map->impl->keys[key_index] = rcutils_strdup(key, allocator); if (NULL == string_map->impl->keys[key_index]) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for key", rcutils_get_default_allocator()) + RCUTILS_SET_ERROR_MSG("failed to allocate memory for key") return RCUTILS_RET_BAD_ALLOC; } should_free_key_on_error = true; @@ -328,7 +308,7 @@ rcutils_string_map_set_no_resize( char * original_value = string_map->impl->values[key_index]; char * new_value = rcutils_strdup(value, allocator); if (NULL == new_value) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for key", allocator) + RCUTILS_SET_ERROR_MSG("failed to allocate memory for key") if (should_free_key_on_error) { allocator.deallocate(string_map->impl->keys[key_index], allocator.state); string_map->impl->keys[key_index] = NULL; @@ -351,16 +331,15 @@ rcutils_ret_t rcutils_string_map_unset(rcutils_string_map_t * string_map, const char * key) { RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) + return RCUTILS_RET_STRING_MAP_INVALID) RCUTILS_CHECK_ARGUMENT_FOR_NULL( - key, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) - rcutils_allocator_t allocator = string_map->impl->allocator; + key, RCUTILS_RET_INVALID_ARGUMENT) size_t key_index; if (!__get_index_of_key_if_exists(string_map->impl, key, strlen(key), &key_index)) { - RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(allocator, "key '%s' not found", key); + RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("key '%s' not found", key); return RCUTILS_RET_STRING_KEY_NOT_FOUND; } __remove_key_and_value_at_index(string_map->impl, key_index); @@ -460,22 +439,18 @@ rcutils_string_map_copy( const rcutils_string_map_t * src_string_map, rcutils_string_map_t * dst_string_map) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - src_string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - dst_string_map, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(src_string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(dst_string_map, RCUTILS_RET_INVALID_ARGUMENT) RCUTILS_CHECK_FOR_NULL_WITH_MSG( - src_string_map->impl, "source string map is invalid", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) + src_string_map->impl, "source string map is invalid", return RCUTILS_RET_STRING_MAP_INVALID) RCUTILS_CHECK_FOR_NULL_WITH_MSG( dst_string_map->impl, "destination string map is invalid", - return RCUTILS_RET_STRING_MAP_INVALID, rcutils_get_default_allocator()) + return RCUTILS_RET_STRING_MAP_INVALID) const char * key = rcutils_string_map_get_next_key(src_string_map, NULL); while (key != NULL) { const char * value = rcutils_string_map_get(src_string_map, key); if (NULL == value) { - RCUTILS_SET_ERROR_MSG( - "unable to get value for known key, should not happen", rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG("unable to get value for known key, should not happen"); return RCUTILS_RET_ERROR; } rcutils_ret_t ret = rcutils_string_map_set(dst_string_map, key, value); diff --git a/src/time.c b/src/time.c index 6b5f2010..42c02330 100644 --- a/src/time.c +++ b/src/time.c @@ -36,14 +36,13 @@ rcutils_time_point_value_as_nanoseconds_string( char * str, size_t str_size) { - rcutils_allocator_t allocator = rcutils_get_default_allocator(); - RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT, allocator) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT, allocator) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT) if (0 == str_size) { return RCUTILS_RET_OK; } if (rcutils_snprintf(str, str_size, "%.19" PRId64, *time_point) < 0) { - RCUTILS_SET_ERROR_MSG("failed to format time point into string as nanoseconds", allocator) + RCUTILS_SET_ERROR_MSG("failed to format time point into string as nanoseconds") return RCUTILS_RET_ERROR; } return RCUTILS_RET_OK; @@ -55,9 +54,8 @@ rcutils_time_point_value_as_seconds_string( char * str, size_t str_size) { - rcutils_allocator_t allocator = rcutils_get_default_allocator(); - RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT, allocator) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT, allocator) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT) if (0 == str_size) { return RCUTILS_RET_OK; } @@ -72,7 +70,7 @@ rcutils_time_point_value_as_seconds_string( str, str_size, "%s%.10" PRId64 ".%.9" PRId64, (*time_point >= 0) ? "" : "-", seconds, nanoseconds) < 0) { - RCUTILS_SET_ERROR_MSG("failed to format time point into string as float seconds", allocator) + RCUTILS_SET_ERROR_MSG("failed to format time point into string as float seconds") return RCUTILS_RET_ERROR; } return RCUTILS_RET_OK; diff --git a/src/time_unix.c b/src/time_unix.c index f3fd108c..028fb453 100644 --- a/src/time_unix.c +++ b/src/time_unix.c @@ -48,8 +48,7 @@ extern "C" rcutils_ret_t rcutils_system_time_now(rcutils_time_point_value_t * now) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - now, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(now, RCUTILS_RET_INVALID_ARGUMENT); struct timespec timespec_now; #if defined(__MACH__) // On OS X use clock_get_time. @@ -65,7 +64,7 @@ rcutils_system_time_now(rcutils_time_point_value_t * now) clock_gettime(CLOCK_REALTIME, ×pec_now); #endif // defined(__MACH__) if (__WOULD_BE_NEGATIVE(timespec_now.tv_sec, timespec_now.tv_nsec)) { - RCUTILS_SET_ERROR_MSG("unexpected negative time", rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG("unexpected negative time"); return RCUTILS_RET_ERROR; } *now = RCUTILS_S_TO_NS((uint64_t)timespec_now.tv_sec) + timespec_now.tv_nsec; @@ -75,8 +74,7 @@ rcutils_system_time_now(rcutils_time_point_value_t * now) rcutils_ret_t rcutils_steady_time_now(rcutils_time_point_value_t * now) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - now, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(now, RCUTILS_RET_INVALID_ARGUMENT); // If clock_gettime is available or on OS X, use a timespec. struct timespec timespec_now; #if defined(__MACH__) @@ -97,7 +95,7 @@ rcutils_steady_time_now(rcutils_time_point_value_t * now) #endif // defined(CLOCK_MONOTONIC_RAW) #endif // defined(__MACH__) if (__WOULD_BE_NEGATIVE(timespec_now.tv_sec, timespec_now.tv_nsec)) { - RCUTILS_SET_ERROR_MSG("unexpected negative time", rcutils_get_default_allocator()); + RCUTILS_SET_ERROR_MSG("unexpected negative time"); return RCUTILS_RET_ERROR; } *now = RCUTILS_S_TO_NS((uint64_t)timespec_now.tv_sec) + timespec_now.tv_nsec; diff --git a/src/time_win32.c b/src/time_win32.c index ab25f1dd..1ab0ac7b 100644 --- a/src/time_win32.c +++ b/src/time_win32.c @@ -32,8 +32,7 @@ extern "C" rcutils_ret_t rcutils_system_time_now(rcutils_time_point_value_t * now) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - now, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(now, RCUTILS_RET_INVALID_ARGUMENT); FILETIME ft; GetSystemTimePreciseAsFileTime(&ft); LARGE_INTEGER li; @@ -50,8 +49,7 @@ rcutils_system_time_now(rcutils_time_point_value_t * now) rcutils_ret_t rcutils_steady_time_now(rcutils_time_point_value_t * now) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - now, RCUTILS_RET_INVALID_ARGUMENT, rcutils_get_default_allocator()); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(now, RCUTILS_RET_INVALID_ARGUMENT); LARGE_INTEGER cpu_frequency, performance_count; // These should not ever fail since XP is already end of life: // From https://msdn.microsoft.com/en-us/library/windows/desktop/ms644905(v=vs.85).aspx and diff --git a/test/test_logging_long_messages.cpp b/test/test_logging_long_messages.cpp index 6501068e..8b802127 100644 --- a/test/test_logging_long_messages.cpp +++ b/test/test_logging_long_messages.cpp @@ -22,7 +22,7 @@ int main(int, char **) { rcutils_ret_t ret = rcutils_logging_initialize(); if (ret != RCUTILS_RET_OK) { - fprintf(stderr, "error initializing logging: %s\n", rcutils_get_error_string_safe()); + fprintf(stderr, "error initializing logging: %s\n", rcutils_get_error_string().str); return -1; } diff --git a/test/test_split.cpp b/test/test_split.cpp index c9a3206a..ada511ac 100644 --- a/test/test_split.cpp +++ b/test/test_split.cpp @@ -69,7 +69,7 @@ TEST(test_split, split) { rcutils_string_array_t tokens1 = test_split("hello_world", '/', 1); EXPECT_STREQ("hello_world", tokens1.data[0]); ret = rcutils_string_array_fini(&tokens1); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; rcutils_string_array_t tokens2 = test_split("hello/world", '/', 2); EXPECT_STREQ("hello", tokens2.data[0]); diff --git a/test/test_string_map.cpp b/test/test_string_map.cpp index 0b201722..5efff838 100644 --- a/test/test_string_map.cpp +++ b/test/test_string_map.cpp @@ -30,27 +30,27 @@ TEST(test_string_map, lifecycle) { { rcutils_string_map_t string_map = rcutils_get_zero_initialized_string_map(); ret = rcutils_string_map_fini(&string_map); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; } // init and then fini and then fini again { rcutils_string_map_t string_map = rcutils_get_zero_initialized_string_map(); ret = rcutils_string_map_init(&string_map, 10, allocator); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_fini(&string_map); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_fini(&string_map); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; } // init and then fini with 0 capacity { rcutils_string_map_t string_map = rcutils_get_zero_initialized_string_map(); ret = rcutils_string_map_init(&string_map, 0, allocator); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_fini(&string_map); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; } // init on non-zero initialized @@ -59,7 +59,7 @@ TEST(test_string_map, lifecycle) { // dirty the memory, otherwise this is flaky (sometimes the junk memory is null) memset(&string_map, 0x7, sizeof(rcutils_string_map_t)); ret = rcutils_string_map_init(&string_map, 10, allocator); - EXPECT_EQ(RCUTILS_RET_STRING_MAP_ALREADY_INIT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_STRING_MAP_ALREADY_INIT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } @@ -67,18 +67,18 @@ TEST(test_string_map, lifecycle) { { rcutils_string_map_t string_map = rcutils_get_zero_initialized_string_map(); ret = rcutils_string_map_init(&string_map, 10, allocator); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_init(&string_map, 10, allocator); - EXPECT_EQ(RCUTILS_RET_STRING_MAP_ALREADY_INIT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_STRING_MAP_ALREADY_INIT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; } // null for string map pointer to init { ret = rcutils_string_map_init(NULL, 10, allocator); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } @@ -86,14 +86,14 @@ TEST(test_string_map, lifecycle) { { rcutils_string_map_t string_map = rcutils_get_zero_initialized_string_map(); ret = rcutils_string_map_init(&string_map, 10, failing_allocator); - EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } // null for string map to fini { ret = rcutils_string_map_fini(NULL); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } } @@ -106,10 +106,10 @@ TEST(test_string_map, getters) { { size_t capacity, size; ret = rcutils_string_map_get_capacity(NULL, &capacity); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_get_size(NULL, &size); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } @@ -120,10 +120,10 @@ TEST(test_string_map, getters) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_get_capacity(&string_map, NULL); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_get_size(&string_map, NULL); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } @@ -410,14 +410,14 @@ TEST(test_string_map, reserve_and_clear) { // null for string_map to reserve { ret = rcutils_string_map_reserve(NULL, 42); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } // null for string_map to clear { ret = rcutils_string_map_clear(NULL); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } } @@ -448,7 +448,7 @@ TEST(test_string_map, set_no_resize) { } ret = rcutils_string_map_set_no_resize(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 1; @@ -467,7 +467,7 @@ TEST(test_string_map, set_no_resize) { } ret = rcutils_string_map_set_no_resize(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_NOT_ENOUGH_SPACE, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_NOT_ENOUGH_SPACE, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -495,7 +495,7 @@ TEST(test_string_map, set_no_resize) { } ret = rcutils_string_map_set_no_resize(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 2; @@ -514,7 +514,7 @@ TEST(test_string_map, set_no_resize) { } ret = rcutils_string_map_set_no_resize(&string_map, "key1", "val1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 2; @@ -533,7 +533,7 @@ TEST(test_string_map, set_no_resize) { } ret = rcutils_string_map_set_no_resize(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 2; @@ -564,7 +564,7 @@ TEST(test_string_map, set_no_resize) { set_failing_allocator_is_failing(failing_allocator, true); ret = rcutils_string_map_set_no_resize(&string_map, "key1", "value1"); - EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -574,7 +574,7 @@ TEST(test_string_map, set_no_resize) { // pass NULL for string_map { ret = rcutils_string_map_set_no_resize(NULL, "key1", "value1"); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } @@ -585,7 +585,7 @@ TEST(test_string_map, set_no_resize) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set_no_resize(&string_map, NULL, "value1"); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -599,7 +599,7 @@ TEST(test_string_map, set_no_resize) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set_no_resize(&string_map, "key1", NULL); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -633,7 +633,7 @@ TEST(test_string_map, set) { } ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 1; @@ -676,7 +676,7 @@ TEST(test_string_map, set) { } ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 1; @@ -695,7 +695,7 @@ TEST(test_string_map, set) { } ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 2; @@ -714,7 +714,7 @@ TEST(test_string_map, set) { } ret = rcutils_string_map_set(&string_map, "key3", "value3"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 4; @@ -757,7 +757,7 @@ TEST(test_string_map, set) { } ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 2; @@ -776,7 +776,7 @@ TEST(test_string_map, set) { } ret = rcutils_string_map_set(&string_map, "key1", "val1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 2; @@ -795,7 +795,7 @@ TEST(test_string_map, set) { } ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 2; @@ -826,7 +826,7 @@ TEST(test_string_map, set) { set_failing_allocator_is_failing(failing_allocator, true); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_BAD_ALLOC, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -836,7 +836,7 @@ TEST(test_string_map, set) { // pass NULL for string_map { ret = rcutils_string_map_set(NULL, "key1", "value1"); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } @@ -847,7 +847,7 @@ TEST(test_string_map, set) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, NULL, "value1"); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -861,7 +861,7 @@ TEST(test_string_map, set) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", NULL); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -884,14 +884,14 @@ TEST(test_string_map, key_exists) { EXPECT_FALSE(key_exists); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; key_exists = rcutils_string_map_key_exists(&string_map, "key1"); EXPECT_TRUE(key_exists); key_exists = rcutils_string_map_key_exists(&string_map, "key2"); EXPECT_FALSE(key_exists); ret = rcutils_string_map_unset(&string_map, "key1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; key_exists = rcutils_string_map_key_exists(&string_map, "key1"); EXPECT_FALSE(key_exists); key_exists = rcutils_string_map_key_exists(&string_map, "key2"); @@ -945,9 +945,9 @@ TEST(test_string_map, key_existsn) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_TRUE(rcutils_string_map_key_existsn(&string_map, "key1andsome", 4)); EXPECT_TRUE(rcutils_string_map_key_existsn(&string_map, "key2andsome", 4)); @@ -983,11 +983,11 @@ TEST(test_string_map, unset) { } ret = rcutils_string_map_set_no_resize(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set_no_resize(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set_no_resize(&string_map, "key3", "value3"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 3; @@ -1008,7 +1008,7 @@ TEST(test_string_map, unset) { } ret = rcutils_string_map_unset(&string_map, "key2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 3; @@ -1028,7 +1028,7 @@ TEST(test_string_map, unset) { } ret = rcutils_string_map_set(&string_map, "key3", "value3.1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 3; @@ -1048,7 +1048,7 @@ TEST(test_string_map, unset) { } ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; { size_t expected = 3; @@ -1075,7 +1075,7 @@ TEST(test_string_map, unset) { // unset with string_map as null { ret = rcutils_string_map_unset(NULL, "key"); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); } @@ -1086,7 +1086,7 @@ TEST(test_string_map, unset) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_unset(&string_map, NULL); - EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_INVALID_ARGUMENT, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -1100,7 +1100,7 @@ TEST(test_string_map, unset) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_unset(&string_map, "missing"); - EXPECT_EQ(RCUTILS_RET_STRING_KEY_NOT_FOUND, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_STRING_KEY_NOT_FOUND, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -1114,12 +1114,12 @@ TEST(test_string_map, unset) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_unset(&string_map, "missing"); - EXPECT_EQ(RCUTILS_RET_STRING_KEY_NOT_FOUND, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_STRING_KEY_NOT_FOUND, ret) << rcutils_get_error_string().str; rcutils_reset_error(); ret = rcutils_string_map_fini(&string_map); @@ -1138,9 +1138,9 @@ TEST(test_string_map, get) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("value1", rcutils_string_map_get(&string_map, "key1")); EXPECT_STREQ("value2", rcutils_string_map_get(&string_map, "key2")); @@ -1156,7 +1156,7 @@ TEST(test_string_map, get) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_EQ(NULL, rcutils_string_map_get(&string_map, "some_key")); @@ -1200,7 +1200,7 @@ TEST(test_string_map, get) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_EQ(NULL, rcutils_string_map_get(&string_map, NULL)); @@ -1220,9 +1220,9 @@ TEST(test_string_map, getn) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("value1", rcutils_string_map_getn(&string_map, "key1andsome", 4)); EXPECT_STREQ("value2", rcutils_string_map_getn(&string_map, "key2andsome", 4)); @@ -1238,7 +1238,7 @@ TEST(test_string_map, getn) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_EQ(NULL, rcutils_string_map_get(&string_map, "some_key")); @@ -1282,7 +1282,7 @@ TEST(test_string_map, getn) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_EQ(NULL, rcutils_string_map_get(&string_map, NULL)); @@ -1302,9 +1302,9 @@ TEST(test_string_map, get_next_key) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; const char * first_key = rcutils_string_map_get_next_key(&string_map, NULL); EXPECT_STREQ("key1", first_key); @@ -1324,9 +1324,9 @@ TEST(test_string_map, get_next_key) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; const char * first_key = rcutils_string_map_get_next_key(&string_map, NULL); EXPECT_STREQ("key1", first_key); @@ -1359,15 +1359,15 @@ TEST(test_string_map, get_next_key) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&string_map, "key3", "value3"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; // removing key2 will create a gap in the map between key1 and key3 ret = rcutils_string_map_unset(&string_map, "key2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; const char * first_key = rcutils_string_map_get_next_key(&string_map, NULL); EXPECT_STREQ("key1", first_key); @@ -1392,9 +1392,9 @@ TEST(test_string_map, copy) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&src_string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&src_string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; rcutils_string_map_t dst_string_map = rcutils_get_zero_initialized_string_map(); ret = rcutils_string_map_init(&dst_string_map, 0, allocator); @@ -1442,9 +1442,9 @@ TEST(test_string_map, copy) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&dst_string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&dst_string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_copy(&src_string_map, &dst_string_map); ASSERT_EQ(RCUTILS_RET_OK, ret); @@ -1465,18 +1465,18 @@ TEST(test_string_map, copy) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&src_string_map, "key1", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&src_string_map, "key2", "value2"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; rcutils_string_map_t dst_string_map = rcutils_get_zero_initialized_string_map(); ret = rcutils_string_map_init(&dst_string_map, 4, allocator); ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&dst_string_map, "key2", "value2.1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_set(&dst_string_map, "key3", "value3"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; ret = rcutils_string_map_copy(&src_string_map, &dst_string_map); ASSERT_EQ(RCUTILS_RET_OK, ret); @@ -1503,7 +1503,7 @@ TEST(test_string_map, strange_keys) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("value1", rcutils_string_map_get(&string_map, "")); @@ -1518,7 +1518,7 @@ TEST(test_string_map, strange_keys) { ASSERT_EQ(RCUTILS_RET_OK, ret); ret = rcutils_string_map_set(&string_map, "key with spaces", "value1"); - ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + ASSERT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("value1", rcutils_string_map_get(&string_map, "key with spaces")); diff --git a/test/test_time.cpp b/test/test_time.cpp index 197abd5c..bb1c67ee 100644 --- a/test/test_time.cpp +++ b/test/test_time.cpp @@ -55,14 +55,14 @@ TEST_F(TestTimeFixture, test_rcutils_system_time_now) { rcutils_ret_t ret; // Check for invalid argument error condition (allowed to alloc). ret = rcutils_system_time_now(nullptr); - EXPECT_EQ(ret, RCUTILS_RET_INVALID_ARGUMENT) << rcutils_get_error_string_safe(); + EXPECT_EQ(ret, RCUTILS_RET_INVALID_ARGUMENT) << rcutils_get_error_string().str; rcutils_reset_error(); // Check for normal operation (not allowed to alloc). rcutils_time_point_value_t now = 0; EXPECT_NO_MEMORY_OPERATIONS({ ret = rcutils_system_time_now(&now); }); - EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string_safe(); + EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string().str; EXPECT_NE(0u, now); // Compare to std::chrono::system_clock time (within a second). now = 0; @@ -82,14 +82,14 @@ TEST_F(TestTimeFixture, test_rcutils_steady_time_now) { rcutils_ret_t ret; // Check for invalid argument error condition (allowed to alloc). ret = rcutils_steady_time_now(nullptr); - EXPECT_EQ(ret, RCUTILS_RET_INVALID_ARGUMENT) << rcutils_get_error_string_safe(); + EXPECT_EQ(ret, RCUTILS_RET_INVALID_ARGUMENT) << rcutils_get_error_string().str; rcutils_reset_error(); // Check for normal operation (not allowed to alloc). rcutils_time_point_value_t now = 0; EXPECT_NO_MEMORY_OPERATIONS({ ret = rcutils_steady_time_now(&now); }); - EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string_safe(); + EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string().str; EXPECT_NE(0u, now); // Compare to std::chrono::steady_clock difference of two times (within a second). now = 0; @@ -97,7 +97,7 @@ TEST_F(TestTimeFixture, test_rcutils_steady_time_now) { ret = rcutils_steady_time_now(&now); }); std::chrono::steady_clock::time_point now_sc = std::chrono::steady_clock::now(); - EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string_safe(); + EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string().str; // Wait for a little while. std::this_thread::sleep_for(std::chrono::milliseconds(100)); // Then take a new timestamp with each and compare. @@ -106,7 +106,7 @@ TEST_F(TestTimeFixture, test_rcutils_steady_time_now) { ret = rcutils_steady_time_now(&later); }); std::chrono::steady_clock::time_point later_sc = std::chrono::steady_clock::now(); - EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string_safe(); + EXPECT_EQ(ret, RCUTILS_RET_OK) << rcutils_get_error_string().str; int64_t steady_diff = later - now; int64_t sc_diff = std::chrono::duration_cast(later_sc - now_sc).count(); @@ -124,7 +124,7 @@ TEST_F(TestTimeFixture, test_rcutils_time_point_value_as_nanoseconds_string) { // Typical use case. timepoint = 100; ret = rcutils_time_point_value_as_nanoseconds_string(&timepoint, buffer, sizeof(buffer)); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("0000000000000000100", buffer); // nullptr for timepoint @@ -141,30 +141,30 @@ TEST_F(TestTimeFixture, test_rcutils_time_point_value_as_nanoseconds_string) { // test truncations timepoint = 100; ret = rcutils_time_point_value_as_nanoseconds_string(&timepoint, buffer, 18); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("00000000000000001", buffer); const char * test_str = "should not be touched"; timepoint = 100; (void)memmove(buffer, test_str, strlen(test_str) + 1); // buffer is of size 256, so it will fit ret = rcutils_time_point_value_as_nanoseconds_string(&timepoint, buffer, 0); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ(test_str, buffer); timepoint = 100; ret = rcutils_time_point_value_as_nanoseconds_string(&timepoint, buffer, 1); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("", buffer); timepoint = 100; ret = rcutils_time_point_value_as_nanoseconds_string(&timepoint, buffer, 3); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("00", buffer); // test negative timepoint = -100; ret = rcutils_time_point_value_as_nanoseconds_string(&timepoint, buffer, sizeof(buffer)); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("-0000000000000000100", buffer); } @@ -177,7 +177,7 @@ TEST_F(TestTimeFixture, test_rcutils_time_point_value_as_seconds_string) { // Typical use case. timepoint = 100; ret = rcutils_time_point_value_as_seconds_string(&timepoint, buffer, sizeof(buffer)); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("0000000000.000000100", buffer); // nullptr for timepoint @@ -194,29 +194,29 @@ TEST_F(TestTimeFixture, test_rcutils_time_point_value_as_seconds_string) { // test truncations timepoint = 100; ret = rcutils_time_point_value_as_seconds_string(&timepoint, buffer, 19); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("0000000000.0000001", buffer); const char * test_str = "should not be touched"; timepoint = 100; (void)memmove(buffer, test_str, strlen(test_str) + 1); // buffer is of size 256, so it will fit ret = rcutils_time_point_value_as_seconds_string(&timepoint, buffer, 0); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ(test_str, buffer); timepoint = 100; ret = rcutils_time_point_value_as_seconds_string(&timepoint, buffer, 1); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("", buffer); timepoint = 100; ret = rcutils_time_point_value_as_seconds_string(&timepoint, buffer, 3); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("00", buffer); // test negative timepoint = -100; ret = rcutils_time_point_value_as_seconds_string(&timepoint, buffer, sizeof(buffer)); - EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string_safe(); + EXPECT_EQ(RCUTILS_RET_OK, ret) << rcutils_get_error_string().str; EXPECT_STREQ("-0000000000.000000100", buffer); } From 086352eb27224635a666621c0e70f93975d763e6 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Mon, 29 Oct 2018 13:17:15 -0500 Subject: [PATCH 03/20] support c99 as well Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index 8177ab6d..3c6d145b 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -56,12 +56,12 @@ extern "C" // e.g. "some error, at /path/to/a.c:42, at /path/to/b.c:42" #define RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH 768 // with RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH = 768, RCUTILS_ERROR_STATE_FILE_MAX_LENGTH == 228 -#define RCUTILS_ERROR_STATE_FILE_MAX_LENGTH ( \ - RCUTILS_ERROR_MESSAGE_MAX_LENGTH - \ - RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH - \ - RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH - \ - RCUTILS_ERROR_FORMATTING_CHARACTERS - \ - 1) +#define RCUTILS_ERROR_STATE_FILE_MAX_LENGTH \ + RCUTILS_ERROR_MESSAGE_MAX_LENGTH - \ + RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH - \ + RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH - \ + RCUTILS_ERROR_FORMATTING_CHARACTERS - \ + 1 /// Struct wrapping a fixed-size c string used for returning the formatted error string. typedef struct rcutils_error_string_t @@ -82,14 +82,16 @@ typedef struct rcutils_error_state_t } rcutils_error_state_t; // make sure our math is right... +#if __STDC_VERSION__ >= 201112L static_assert( - RCUTILS_ERROR_MESSAGE_MAX_LENGTH == ( + sizeof(rcutils_error_string_t) == ( RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH + RCUTILS_ERROR_STATE_FILE_MAX_LENGTH + RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH + RCUTILS_ERROR_FORMATTING_CHARACTERS + 1 /* null terminating character */), "Maximum length calculations incorrect"); +#endif /// Forces initialization of thread-local storage if called in a newly created thread. /** From efc9ac802c199e78f240eb7e512ce82d40a90ab2 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Mon, 29 Oct 2018 22:30:55 -0500 Subject: [PATCH 04/20] remove const in strings Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index 3c6d145b..be5a0b49 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -66,17 +66,17 @@ extern "C" /// Struct wrapping a fixed-size c string used for returning the formatted error string. typedef struct rcutils_error_string_t { - const char str[RCUTILS_ERROR_MESSAGE_MAX_LENGTH]; + char str[RCUTILS_ERROR_MESSAGE_MAX_LENGTH]; } rcutils_error_string_t; /// Struct which encapsulates the error state set by RCUTILS_SET_ERROR_MSG(). typedef struct rcutils_error_state_t { /// User message storage, limited to RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH characters. - const char message[RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH]; + char message[RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH]; /// File name, limited to what's left from RCUTILS_ERROR_STATE_MAX_SIZE characters /// after subtracting storage for others. - const char file[RCUTILS_ERROR_STATE_FILE_MAX_LENGTH]; + char file[RCUTILS_ERROR_STATE_FILE_MAX_LENGTH]; /// Line number of error. uint64_t line_number; } rcutils_error_state_t; From d6aa6676a8ce1c8eb5abc702cd2672debcd83f31 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Mon, 29 Oct 2018 22:35:22 -0500 Subject: [PATCH 05/20] remove use of const casting Signed-off-by: William Woodall --- src/error_handling.c | 4 ++-- src/error_handling_helpers.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/error_handling.c b/src/error_handling.c index b4a95bfc..10ba6cbf 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -86,8 +86,8 @@ rcutils_set_error_state( rcutils_error_state_t error_state; // The const is casted away to set the 'message' field of the error state (const for users only). - __rcutils_copy_string((char *)error_state.message, sizeof(error_state.message), error_string); - __rcutils_copy_string((char *)error_state.file, sizeof(error_state.file), file); + __rcutils_copy_string(error_state.message, sizeof(error_state.message), error_string); + __rcutils_copy_string(error_state.file, sizeof(error_state.file), file); error_state.line_number = line_number; #if RCUTILS_REPORT_ERROR_HANDLING_ERRORS // Only warn of overwritting if the new error is different from the old ones. diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index 63f0fdfb..f228ce8a 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -133,7 +133,7 @@ __rcutils_format_error_string( sizeof(line_number_buffer) - 1 /* minus the null-term */ + 1 // null terminator ), "math error in static string formatting"); - char * offset = (char *)error_string->str; + char * offset = error_string->str; size_t bytes_left = sizeof(error_string->str); size_t written = __rcutils_copy_string(offset, bytes_left, error_state->message); offset += written; From 736d40696c178e73df3d8cfaa66a4d3b07c7807b Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 18:46:47 -0500 Subject: [PATCH 06/20] fix warning Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 2 ++ src/error_handling_helpers.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index be5a0b49..e40444b3 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -22,7 +22,9 @@ extern "C" { #endif +#ifndef __STDC_WANT_LIB_EXT1__ #define __STDC_WANT_LIB_EXT1__ 1 // indicate we would like strnlen_s if available +#endif #include #include #include diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index f228ce8a..2f209ad5 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -31,7 +31,9 @@ #define RCUTILS_WARN_ON_TRUNCATION 1 #endif +#ifndef __STDC_WANT_LIB_EXT1__ #define __STDC_WANT_LIB_EXT1__ 1 // indicate we would like memmove_s if available +#endif #include #include #include From 218a99d010088365a2aff123aa95210c5eebb1df Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 18:47:26 -0500 Subject: [PATCH 07/20] fix docs and typos Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 23 +++++++++++------------ src/error_handling_helpers.h | 2 +- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index e40444b3..72e76588 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -102,15 +102,14 @@ static_assert( * allocator will be used to allocate thread-local storage. * * This function may or may not allocate memory. - * The system's thread-local storage implementation may need to - * allocate memory (usually no way of knowing how much storage is needed if you - * cannot know how many threads will be created). - * Most implementations (e.g. C11, C++11, and pthread) do not allow you to + * The system's thread-local storage implementation may need to allocate + * memory, since it usually has no way of knowing how much storage is needed + * without knowing how many threads will be created. + * Most implementations (e.g. C11, C++11, and pthread) do not have ways to * specify how this memory is allocated, but if the implementation allows, the - * given allocator to this function will be used, otherwise it is unused. - * This isn't typically an issue since the memory is only free'd on thread - * destruction, and people trying to avoid memory allocation will also be - * avoiding thread creation and destruction. + * given allocator to this function will be used, but is otherwise unused. + * This only occurs when creating and destroying threads, which can be avoided + * in the "steady" state by reusing pools of threads. * * It is worth considering that repeated thread creation and destruction will * result in repeated memory allocations and could result in memory @@ -121,7 +120,7 @@ static_assert( * been set. * * If called more than once in a thread, or after implicitly initialized by - * setting the error state, it will still return `RCUTILS_RET_OK`, and even + * setting the error state, it will still return `RCUTILS_RET_OK`, even * if the given allocator is invalid. * Essentially this function does nothing if thread-local storage has already * been called. @@ -239,9 +238,9 @@ rcutils_get_error_state(void); /** * This function is "safe" because it returns a copy of the current error * string or one containing the string "error not set" if no error was set. - * This ensures your copy is owned by you and is never invalidated by error - * handling calls, and that the c string inside is always valid and null - * terminated. + * This ensures that the copy is owned by the calling thread and is therefore + * never invalidated by other error handling calls, and that the C string + * inside is always valid and null terminated. * * \return The current error string, with file and line number, or "error not set" if not set. */ diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index 2f209ad5..163e8502 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -105,7 +105,7 @@ __rcutils_convert_uint64_t_into_c_str(uint64_t number, char * buffer) return; } - // add the modulo 10 to thes string and then integer divide by 10 until 0 + // add the modulo 10 to the string and then integer divide by 10 until 0 while (number != 0) { buffer[i++] = number % 10 + '0'; number = number / 10; From ac88bfd6a02f2636e1a6bd7200d38fc69e0d2f4b Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 18:47:43 -0500 Subject: [PATCH 08/20] fix compiler error due to missing include Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index 72e76588..2d734784 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -28,6 +28,7 @@ extern "C" #include #include #include +#include #include #include #include From 0ca66020536660b3d53fc09fb7c619472b796654 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 18:48:26 -0500 Subject: [PATCH 09/20] wrap macros in do { } while(0), refactors in subsequent commits Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index 2d734784..85584d54 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -42,9 +42,11 @@ extern "C" #ifdef __STDC_LIB_EXT1__ // Limit the buffer size in the `fwrite` call to give an upper bound to buffer overrun in the case // of non-null terminated `msg`. -#define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) fwrite(msg, sizeof(char), strnlen_s(msg, 4096), stderr) +#define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) \ + do { fwrite(msg, sizeof(char), strnlen_s(msg, 4096), stderr); } while(0) #else -#define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) fwrite(msg, sizeof(char), strlen(msg), stderr) +#define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) \ + do { fwrite(msg, sizeof(char), strlen(msg), stderr); } while(0) #endif // fixed constraints @@ -178,10 +180,12 @@ rcutils_set_error_state(const char * error_string, const char * file, size_t lin * \param[in] error_statement The statement to evaluate if `value` is `NULL`. */ #define RCUTILS_CHECK_FOR_NULL_WITH_MSG(value, msg, error_statement) \ - if (NULL == value) { \ - RCUTILS_SET_ERROR_MSG(msg); \ - error_statement; \ - } + do { \ + if (NULL == value) { \ + RCUTILS_SET_ERROR_MSG(msg); \ + error_statement; \ + } \ + } while(0) /// Set the error message, as well as append the current file and line number. /** @@ -193,7 +197,8 @@ rcutils_set_error_state(const char * error_string, const char * file, size_t lin * * \param[in] msg The error message to be set. */ -#define RCUTILS_SET_ERROR_MSG(msg) rcutils_set_error_state(msg, __FILE__, __LINE__); +#define RCUTILS_SET_ERROR_MSG(msg) \ + do { rcutils_set_error_state(msg, __FILE__, __LINE__); } while(0) /// Set the error message using a format string and format arguments. /** @@ -213,7 +218,7 @@ rcutils_set_error_state(const char * error_string, const char * file, size_t lin } else { \ RCUTILS_SET_ERROR_MSG(output_msg); \ } \ - } while (false) + } while (0) /// Return `true` if the error is set, otherwise `false`. RCUTILS_PUBLIC From 30cd5cbce21f579876602e6c0bb0c0a62f8b2bf2 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 18:48:55 -0500 Subject: [PATCH 10/20] use semicolons after macros Signed-off-by: William Woodall --- include/rcutils/allocator.h | 2 +- src/error_handling.c | 2 +- src/split.c | 2 +- src/string_array.c | 10 +++--- src/string_map.c | 71 ++++++++++++++++++------------------- src/time.c | 12 +++---- 6 files changed, 48 insertions(+), 51 deletions(-) diff --git a/include/rcutils/allocator.h b/include/rcutils/allocator.h index d92b9f2a..8c677e80 100644 --- a/include/rcutils/allocator.h +++ b/include/rcutils/allocator.h @@ -122,7 +122,7 @@ rcutils_allocator_is_valid(const rcutils_allocator_t * allocator); #define RCUTILS_CHECK_ALLOCATOR_WITH_MSG(allocator, msg, fail_statement) \ if (!rcutils_allocator_is_valid(allocator)) { \ - RCUTILS_SET_ERROR_MSG(msg) \ + RCUTILS_SET_ERROR_MSG(msg); \ fail_statement; \ } diff --git a/src/error_handling.c b/src/error_handling.c index 10ba6cbf..876ceec1 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -60,7 +60,7 @@ rcutils_initialize_error_handling_thread_local_storage(rcutils_allocator_t alloc // to initialize and do any required memory allocation gtls_rcutils_thread_local_initialized = true; rcutils_reset_error(); - RCUTILS_SET_ERROR_MSG("no error - initializing thread-local storage") + RCUTILS_SET_ERROR_MSG("no error - initializing thread-local storage"); { // this scope is to prevent uncrustify from moving the (void) to the previous line (void)rcutils_get_error_string(); } diff --git a/src/split.c b/src/split.c index 6addd42d..b8d02cb4 100644 --- a/src/split.c +++ b/src/split.c @@ -35,7 +35,7 @@ rcutils_split( rcutils_string_array_t * string_array) { if (NULL == string_array) { - RCUTILS_SET_ERROR_MSG("string_array is null") + RCUTILS_SET_ERROR_MSG("string_array is null"); return RCUTILS_RET_INVALID_ARGUMENT; } if (NULL == str || strlen(str) == 0) { diff --git a/src/string_array.c b/src/string_array.c index 0085ab27..bea53bbd 100644 --- a/src/string_array.c +++ b/src/string_array.c @@ -42,17 +42,17 @@ rcutils_string_array_init( const rcutils_allocator_t * allocator) { if (NULL == allocator) { - RCUTILS_SET_ERROR_MSG("allocator is null") + RCUTILS_SET_ERROR_MSG("allocator is null"); return RCUTILS_RET_INVALID_ARGUMENT; } if (NULL == string_array) { - RCUTILS_SET_ERROR_MSG("string_array is null") + RCUTILS_SET_ERROR_MSG("string_array is null"); return RCUTILS_RET_INVALID_ARGUMENT; } string_array->size = size; string_array->data = allocator->zero_allocate(size, sizeof(char *), allocator->state); if (NULL == string_array->data) { - RCUTILS_SET_ERROR_MSG("failed to allocator string array") + RCUTILS_SET_ERROR_MSG("failed to allocator string array"); return RCUTILS_RET_BAD_ALLOC; } string_array->allocator = *allocator; @@ -63,7 +63,7 @@ rcutils_ret_t rcutils_string_array_fini(rcutils_string_array_t * string_array) { if (NULL == string_array) { - RCUTILS_SET_ERROR_MSG("string_array is null") + RCUTILS_SET_ERROR_MSG("string_array is null"); return RCUTILS_RET_INVALID_ARGUMENT; } @@ -73,7 +73,7 @@ rcutils_string_array_fini(rcutils_string_array_t * string_array) rcutils_allocator_t * allocator = &string_array->allocator; if (!rcutils_allocator_is_valid(allocator)) { - RCUTILS_SET_ERROR_MSG("allocator is invalid") + RCUTILS_SET_ERROR_MSG("allocator is invalid"); return RCUTILS_RET_INVALID_ARGUMENT; } size_t i; diff --git a/src/string_map.c b/src/string_map.c index 8ebc560e..ff93baa3 100644 --- a/src/string_map.c +++ b/src/string_map.c @@ -52,16 +52,16 @@ rcutils_string_map_init( size_t initial_capacity, rcutils_allocator_t allocator) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); if (string_map->impl != NULL) { - RCUTILS_SET_ERROR_MSG("string_map already initialized") + RCUTILS_SET_ERROR_MSG("string_map already initialized"); return RCUTILS_RET_STRING_MAP_ALREADY_INIT; } RCUTILS_CHECK_ALLOCATOR_WITH_MSG( &allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT) string_map->impl = allocator.allocate(sizeof(rcutils_string_map_impl_t), allocator.state); if (NULL == string_map->impl) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for string map impl struct") + RCUTILS_SET_ERROR_MSG("failed to allocate memory for string map impl struct"); return RCUTILS_RET_BAD_ALLOC; } string_map->impl->keys = NULL; @@ -82,7 +82,7 @@ rcutils_string_map_init( rcutils_ret_t rcutils_string_map_fini(rcutils_string_map_t * string_map) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); if (NULL == string_map->impl) { return RCUTILS_RET_OK; } @@ -107,10 +107,10 @@ rcutils_string_map_fini(rcutils_string_map_t * string_map) rcutils_ret_t rcutils_string_map_get_capacity(const rcutils_string_map_t * string_map, size_t * capacity) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(capacity, RCUTILS_RET_INVALID_ARGUMENT) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(capacity, RCUTILS_RET_INVALID_ARGUMENT); // *INDENT-OFF* (prevent uncrustify getting this wrong) *capacity = string_map->impl->capacity; // *INDENT-ON* @@ -120,10 +120,10 @@ rcutils_string_map_get_capacity(const rcutils_string_map_t * string_map, size_t rcutils_ret_t rcutils_string_map_get_size(const rcutils_string_map_t * string_map, size_t * size) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(size, RCUTILS_RET_INVALID_ARGUMENT) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(size, RCUTILS_RET_INVALID_ARGUMENT); { // otherwise uncrustify moves the * to the end of the previous line... *size = string_map->impl->size; } @@ -133,9 +133,9 @@ rcutils_string_map_get_size(const rcutils_string_map_t * string_map, size_t * si rcutils_ret_t rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID); rcutils_allocator_t allocator = string_map->impl->allocator; // short circuit, if requested capacity is less than the size of the map if (capacity < string_map->impl->size) { @@ -159,7 +159,7 @@ rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) // ensure that reallocate won't overflow capacity if (capacity > (SIZE_MAX / sizeof(char *))) { - RCUTILS_SET_ERROR_MSG("requested capacity for string_map too large") + RCUTILS_SET_ERROR_MSG("requested capacity for string_map too large"); return RCUTILS_RET_BAD_ALLOC; } @@ -167,7 +167,7 @@ rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) char ** new_keys = allocator.reallocate(string_map->impl->keys, capacity * sizeof(char *), allocator.state); if (NULL == new_keys) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map keys") + RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map keys"); return RCUTILS_RET_BAD_ALLOC; } string_map->impl->keys = new_keys; @@ -176,7 +176,7 @@ rcutils_string_map_reserve(rcutils_string_map_t * string_map, size_t capacity) char ** new_values = allocator.reallocate(string_map->impl->values, capacity * sizeof(char *), allocator.state); if (NULL == new_values) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map values") + RCUTILS_SET_ERROR_MSG("failed to allocate memory for string_map values"); return RCUTILS_RET_BAD_ALLOC; } string_map->impl->values = new_values; @@ -209,9 +209,9 @@ __remove_key_and_value_at_index(rcutils_string_map_impl_t * string_map_impl, siz rcutils_ret_t rcutils_string_map_clear(rcutils_string_map_t * string_map) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID); size_t i = 0; for (; i < string_map->impl->capacity; ++i) { if (string_map->impl->keys[i] != NULL) { @@ -224,11 +224,11 @@ rcutils_string_map_clear(rcutils_string_map_t * string_map) rcutils_ret_t rcutils_string_map_set(rcutils_string_map_t * string_map, const char * key, const char * value) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(key, RCUTILS_RET_INVALID_ARGUMENT) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(key, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT); rcutils_ret_t ret = rcutils_string_map_set_no_resize(string_map, key, value); // if it fails due to not enough space, resize and try again if (ret == RCUTILS_RET_NOT_ENOUGH_SPACE) { @@ -276,11 +276,11 @@ rcutils_string_map_set_no_resize( const char * key, const char * value) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(key, RCUTILS_RET_INVALID_ARGUMENT) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(key, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(value, RCUTILS_RET_INVALID_ARGUMENT); rcutils_allocator_t allocator = string_map->impl->allocator; size_t key_index; bool should_free_key_on_error = false; @@ -299,7 +299,7 @@ rcutils_string_map_set_no_resize( assert(key_index < string_map->impl->capacity); // defensive, this should not happen string_map->impl->keys[key_index] = rcutils_strdup(key, allocator); if (NULL == string_map->impl->keys[key_index]) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for key") + RCUTILS_SET_ERROR_MSG("failed to allocate memory for key"); return RCUTILS_RET_BAD_ALLOC; } should_free_key_on_error = true; @@ -308,7 +308,7 @@ rcutils_string_map_set_no_resize( char * original_value = string_map->impl->values[key_index]; char * new_value = rcutils_strdup(value, allocator); if (NULL == new_value) { - RCUTILS_SET_ERROR_MSG("failed to allocate memory for key") + RCUTILS_SET_ERROR_MSG("failed to allocate memory for key"); if (should_free_key_on_error) { allocator.deallocate(string_map->impl->keys[key_index], allocator.state); string_map->impl->keys[key_index] = NULL; @@ -330,13 +330,10 @@ rcutils_string_map_set_no_resize( rcutils_ret_t rcutils_string_map_unset(rcutils_string_map_t * string_map, const char * key) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - string_map->impl, "invalid string map", - return RCUTILS_RET_STRING_MAP_INVALID) - RCUTILS_CHECK_ARGUMENT_FOR_NULL( - key, RCUTILS_RET_INVALID_ARGUMENT) + string_map->impl, "invalid string map", return RCUTILS_RET_STRING_MAP_INVALID); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(key, RCUTILS_RET_INVALID_ARGUMENT); size_t key_index; if (!__get_index_of_key_if_exists(string_map->impl, key, strlen(key), &key_index)) { RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING("key '%s' not found", key); @@ -439,13 +436,13 @@ rcutils_string_map_copy( const rcutils_string_map_t * src_string_map, rcutils_string_map_t * dst_string_map) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(src_string_map, RCUTILS_RET_INVALID_ARGUMENT) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(dst_string_map, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(src_string_map, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(dst_string_map, RCUTILS_RET_INVALID_ARGUMENT); RCUTILS_CHECK_FOR_NULL_WITH_MSG( - src_string_map->impl, "source string map is invalid", return RCUTILS_RET_STRING_MAP_INVALID) + src_string_map->impl, "source string map is invalid", return RCUTILS_RET_STRING_MAP_INVALID); RCUTILS_CHECK_FOR_NULL_WITH_MSG( dst_string_map->impl, "destination string map is invalid", - return RCUTILS_RET_STRING_MAP_INVALID) + return RCUTILS_RET_STRING_MAP_INVALID); const char * key = rcutils_string_map_get_next_key(src_string_map, NULL); while (key != NULL) { const char * value = rcutils_string_map_get(src_string_map, key); diff --git a/src/time.c b/src/time.c index 42c02330..1706e829 100644 --- a/src/time.c +++ b/src/time.c @@ -36,13 +36,13 @@ rcutils_time_point_value_as_nanoseconds_string( char * str, size_t str_size) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT); if (0 == str_size) { return RCUTILS_RET_OK; } if (rcutils_snprintf(str, str_size, "%.19" PRId64, *time_point) < 0) { - RCUTILS_SET_ERROR_MSG("failed to format time point into string as nanoseconds") + RCUTILS_SET_ERROR_MSG("failed to format time point into string as nanoseconds"); return RCUTILS_RET_ERROR; } return RCUTILS_RET_OK; @@ -54,8 +54,8 @@ rcutils_time_point_value_as_seconds_string( char * str, size_t str_size) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT) - RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT) + RCUTILS_CHECK_ARGUMENT_FOR_NULL(time_point, RCUTILS_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ARGUMENT_FOR_NULL(str, RCUTILS_RET_INVALID_ARGUMENT); if (0 == str_size) { return RCUTILS_RET_OK; } @@ -70,7 +70,7 @@ rcutils_time_point_value_as_seconds_string( str, str_size, "%s%.10" PRId64 ".%.9" PRId64, (*time_point >= 0) ? "" : "-", seconds, nanoseconds) < 0) { - RCUTILS_SET_ERROR_MSG("failed to format time point into string as float seconds") + RCUTILS_SET_ERROR_MSG("failed to format time point into string as float seconds"); return RCUTILS_RET_ERROR; } return RCUTILS_RET_OK; From c1d32edc79afb3f7fbd9272240aa1f02baf03616 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 19:23:33 -0500 Subject: [PATCH 11/20] uncrustify Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index 85584d54..b1accd34 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -43,10 +43,10 @@ extern "C" // Limit the buffer size in the `fwrite` call to give an upper bound to buffer overrun in the case // of non-null terminated `msg`. #define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) \ - do { fwrite(msg, sizeof(char), strnlen_s(msg, 4096), stderr); } while(0) + do {fwrite(msg, sizeof(char), strnlen_s(msg, 4096), stderr);} while (0) #else #define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) \ - do { fwrite(msg, sizeof(char), strlen(msg), stderr); } while(0) + do {fwrite(msg, sizeof(char), strlen(msg), stderr);} while (0) #endif // fixed constraints @@ -185,7 +185,7 @@ rcutils_set_error_state(const char * error_string, const char * file, size_t lin RCUTILS_SET_ERROR_MSG(msg); \ error_statement; \ } \ - } while(0) + } while (0) /// Set the error message, as well as append the current file and line number. /** @@ -198,7 +198,7 @@ rcutils_set_error_state(const char * error_string, const char * file, size_t lin * \param[in] msg The error message to be set. */ #define RCUTILS_SET_ERROR_MSG(msg) \ - do { rcutils_set_error_state(msg, __FILE__, __LINE__); } while(0) + do {rcutils_set_error_state(msg, __FILE__, __LINE__);} while (0) /// Set the error message using a format string and format arguments. /** From 622740faf4da44b446111b032f91a0320ca7d3fc Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 19:24:04 -0500 Subject: [PATCH 12/20] typo Signed-off-by: William Woodall --- src/error_handling.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/error_handling.c b/src/error_handling.c index 876ceec1..5a8ca41a 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -85,7 +85,6 @@ rcutils_set_error_state( { rcutils_error_state_t error_state; - // The const is casted away to set the 'message' field of the error state (const for users only). __rcutils_copy_string(error_state.message, sizeof(error_state.message), error_string); __rcutils_copy_string(error_state.file, sizeof(error_state.file), file); error_state.line_number = line_number; From 659a63b707c7a653f4c0c875bd6e626b8e1b9038 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Tue, 30 Oct 2018 19:24:41 -0500 Subject: [PATCH 13/20] make error handling helpers static, mark as internal, and assert assumptions Signed-off-by: William Woodall --- src/error_handling_helpers.h | 24 ++++++++++++++++++++---- test/test_error_handling_helpers.cpp | 8 ++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index 163e8502..52c787af 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -45,9 +45,14 @@ extern "C" { #endif +// do not use externally, internal function which is only to be used by error_handling.c +static size_t __rcutils_copy_string(char * dst, size_t dst_size, const char * src) { + assert(dst != NULL); + assert(dst_size > 0); + assert(src != NULL); // doesn't matter how long src actually is if it is longer than dst, so limit to dst + 1 size_t src_length = strnlen(src, dst_size + 1); size_t size_to_copy = src_length; @@ -80,9 +85,12 @@ __rcutils_copy_string(char * dst, size_t dst_size, const char * src) return size_to_copy; } +// do not use externally, internal function which is only to be used by error_handling.c +static void __rcutils_reverse_str(char * string_in, size_t string_len) { + assert(string_in != NULL); size_t i = 0; size_t j = string_len - 1; for (; i < j; i++, j--) { @@ -92,10 +100,13 @@ __rcutils_reverse_str(char * string_in, size_t string_len) } } -// sizeof(buffer) must be greater than or equal to 21. +// do not use externally, internal function which is only to be used by error_handling.c +static void -__rcutils_convert_uint64_t_into_c_str(uint64_t number, char * buffer) +__rcutils_convert_uint64_t_into_c_str(uint64_t number, char * buffer, size_t buffer_size) { + assert(buffer != NULL); + assert(buffer_size >= 21); size_t i = 0; // if number is 0, short circuit @@ -118,14 +129,18 @@ __rcutils_convert_uint64_t_into_c_str(uint64_t number, char * buffer) __rcutils_reverse_str(buffer, strnlen(buffer, 21)); } +// do not use externally, internal function which is only to be used by error_handling.c +static void __rcutils_format_error_string( rcutils_error_string_t * error_string, const rcutils_error_state_t * error_state) { + assert(error_string != NULL); + assert(error_state != NULL); static const char format_1[] = ", at "; static const char format_2[] = ":"; - static char line_number_buffer[21]; + char line_number_buffer[21]; static_assert( sizeof(error_string->str) == ( sizeof(error_state->message) + @@ -149,7 +164,8 @@ __rcutils_format_error_string( written = __rcutils_copy_string(offset, bytes_left, format_2); offset += written; bytes_left -= written; - __rcutils_convert_uint64_t_into_c_str(error_state->line_number, line_number_buffer); + __rcutils_convert_uint64_t_into_c_str( + error_state->line_number, line_number_buffer, sizeof(line_number_buffer)); written = __rcutils_copy_string(offset, bytes_left, line_number_buffer); offset += written; offset[0] = '\0'; diff --git a/test/test_error_handling_helpers.cpp b/test/test_error_handling_helpers.cpp index ba0f8f65..585b2742 100644 --- a/test/test_error_handling_helpers.cpp +++ b/test/test_error_handling_helpers.cpp @@ -102,7 +102,7 @@ TEST(test_error_handling, convert_uint64_t_into_c_str) { uint64_t number = UINT64_MAX; char buffer[21]; EXPECT_NO_MEMORY_OPERATIONS({ - __rcutils_convert_uint64_t_into_c_str(number, buffer); + __rcutils_convert_uint64_t_into_c_str(number, buffer, sizeof(buffer)); }); EXPECT_STREQ(buffer, "18446744073709551615"); } @@ -110,7 +110,7 @@ TEST(test_error_handling, convert_uint64_t_into_c_str) { uint64_t number = 0; char buffer[21]; EXPECT_NO_MEMORY_OPERATIONS({ - __rcutils_convert_uint64_t_into_c_str(number, buffer); + __rcutils_convert_uint64_t_into_c_str(number, buffer, sizeof(buffer)); }); EXPECT_STREQ(buffer, "0"); } @@ -118,7 +118,7 @@ TEST(test_error_handling, convert_uint64_t_into_c_str) { uint64_t number = 42; char buffer[21]; EXPECT_NO_MEMORY_OPERATIONS({ - __rcutils_convert_uint64_t_into_c_str(number, buffer); + __rcutils_convert_uint64_t_into_c_str(number, buffer, sizeof(buffer)); }); EXPECT_STREQ(buffer, "42"); } @@ -126,7 +126,7 @@ TEST(test_error_handling, convert_uint64_t_into_c_str) { uint64_t number = INT64_MAX; char buffer[21]; EXPECT_NO_MEMORY_OPERATIONS({ - __rcutils_convert_uint64_t_into_c_str(number, buffer); + __rcutils_convert_uint64_t_into_c_str(number, buffer, sizeof(buffer)); }); EXPECT_STREQ(buffer, "9223372036854775807"); } From cd4f516b9a12e7fe7deb9011de812e4d20e575fe Mon Sep 17 00:00:00 2001 From: William Woodall Date: Wed, 31 Oct 2018 17:19:58 -0500 Subject: [PATCH 14/20] fix warnings and errors Signed-off-by: William Woodall --- src/error_handling.c | 5 ++--- test/test_error_handling.cpp | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/error_handling.c b/src/error_handling.c index 5a8ca41a..b4d8f332 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -61,9 +61,8 @@ rcutils_initialize_error_handling_thread_local_storage(rcutils_allocator_t alloc gtls_rcutils_thread_local_initialized = true; rcutils_reset_error(); RCUTILS_SET_ERROR_MSG("no error - initializing thread-local storage"); - { // this scope is to prevent uncrustify from moving the (void) to the previous line - (void)rcutils_get_error_string(); - } + rcutils_error_string_t throw_away = rcutils_get_error_string(); + (void)throw_away; rcutils_reset_error(); // at this point the thread-local allocator, error state, and error string are all initialized diff --git a/test/test_error_handling.cpp b/test/test_error_handling.cpp index 232abfa3..4ee9b42e 100644 --- a/test/test_error_handling.cpp +++ b/test/test_error_handling.cpp @@ -131,7 +131,7 @@ TEST(test_error_handling, reset) { EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); rcutils_error_string_t error_string = rcutils_get_error_string(); EXPECT_NO_MEMORY_OPERATIONS_END(); - ASSERT_NE(nullptr, error_string.str); + ASSERT_STRNE("", error_string.str); } { EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); @@ -156,7 +156,7 @@ TEST(test_error_handling, empty) { EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); rcutils_error_string_t error_string = rcutils_get_error_string(); EXPECT_NO_MEMORY_OPERATIONS_END(); - ASSERT_NE(nullptr, error_string.str); + ASSERT_STRNE("", error_string.str); } { EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); From 1e54e41756e669fe7f3d99284a3d37aab38c4e9e Mon Sep 17 00:00:00 2001 From: William Woodall Date: Wed, 31 Oct 2018 21:03:00 -0500 Subject: [PATCH 15/20] trying to suppress warning on macOS --- src/error_handling.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/error_handling.c b/src/error_handling.c index b4d8f332..64b2796e 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -14,6 +14,11 @@ // Note: migrated from rmw/error_handling.c in 2017-04 +#ifdef __cplusplus +extern "C" +{ +#endif + #include #include @@ -159,3 +164,7 @@ rcutils_reset_error(void) gtls_rcutils_error_string = (const rcutils_error_string_t) {0}; gtls_rcutils_error_is_set = false; } + +#ifdef __cplusplus +} +#endif From 31479bfc59c5c21ab12fe8bea257da00255afa0b Mon Sep 17 00:00:00 2001 From: William Woodall Date: Wed, 31 Oct 2018 21:15:24 -0500 Subject: [PATCH 16/20] trying to suppress warning on macOS --- src/error_handling.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/error_handling.c b/src/error_handling.c index 64b2796e..85fe35bc 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -126,7 +126,9 @@ rcutils_set_error_state( #endif gtls_rcutils_error_state = error_state; gtls_rcutils_error_string_is_formatted = false; - gtls_rcutils_error_string = (const rcutils_error_string_t) {0}; + gtls_rcutils_error_string = (const rcutils_error_string_t) { + .str = "\0" + }; gtls_rcutils_error_is_set = true; } @@ -161,7 +163,9 @@ rcutils_reset_error(void) .message = {0}, .file = {0}, .line_number = 0 }; // NOLINT(readability/braces) gtls_rcutils_error_string_is_formatted = false; - gtls_rcutils_error_string = (const rcutils_error_string_t) {0}; + gtls_rcutils_error_string = (const rcutils_error_string_t) { + .str = "\0" + }; gtls_rcutils_error_is_set = false; } From 7489b615646ab7761263fed71ec971e0fce1d6d9 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Thu, 1 Nov 2018 14:26:35 -0500 Subject: [PATCH 17/20] handle zero str length in reverse string Signed-off-by: William Woodall --- src/error_handling_helpers.h | 3 +++ test/test_error_handling_helpers.cpp | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index 52c787af..7a9bc2bf 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -91,6 +91,9 @@ void __rcutils_reverse_str(char * string_in, size_t string_len) { assert(string_in != NULL); + if (string_len == 0) { + return; + } size_t i = 0; size_t j = string_len - 1; for (; i < j; i++, j--) { diff --git a/test/test_error_handling_helpers.cpp b/test/test_error_handling_helpers.cpp index 585b2742..3e3a5c17 100644 --- a/test/test_error_handling_helpers.cpp +++ b/test/test_error_handling_helpers.cpp @@ -94,6 +94,13 @@ TEST(test_error_handling, reverse_str) { }); EXPECT_STREQ(buffer, "vererseme"); } + { + char buffer[] = "doesntmatter"; + EXPECT_NO_MEMORY_OPERATIONS({ + __rcutils_reverse_str(buffer, 0); + }); + EXPECT_STREQ(buffer, "doesntmatter"); + } } TEST(test_error_handling, convert_uint64_t_into_c_str) { From 38e504dc507c84925ec02706ecae7b4e87d0eb67 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Thu, 1 Nov 2018 18:35:55 -0500 Subject: [PATCH 18/20] address review feedback Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 6 +-- src/error_handling.c | 21 ++++++++++ src/error_handling_helpers.h | 6 +-- test/test_error_handling.cpp | 57 +++++++++++++++++++++++++--- test/test_error_handling_helpers.cpp | 2 +- 5 files changed, 80 insertions(+), 12 deletions(-) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index b1accd34..6e05e93a 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -60,13 +60,13 @@ extern "C" // remember "chained" errors will include previously specified file paths // e.g. "some error, at /path/to/a.c:42, at /path/to/b.c:42" #define RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH 768 -// with RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH = 768, RCUTILS_ERROR_STATE_FILE_MAX_LENGTH == 228 -#define RCUTILS_ERROR_STATE_FILE_MAX_LENGTH \ +// with RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH = 768, RCUTILS_ERROR_STATE_FILE_MAX_LENGTH == 229 +#define RCUTILS_ERROR_STATE_FILE_MAX_LENGTH (\ RCUTILS_ERROR_MESSAGE_MAX_LENGTH - \ RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH - \ RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH - \ RCUTILS_ERROR_FORMATTING_CHARACTERS - \ - 1 + 1) /// Struct wrapping a fixed-size c string used for returning the formatted error string. typedef struct rcutils_error_string_t diff --git a/src/error_handling.c b/src/error_handling.c index 85fe35bc..74eb2bbb 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -78,6 +78,8 @@ static bool __same_string(const char * str1, const char * str2, size_t count) { + assert(NULL != str1); + assert(NULL != str2); return str1 == str2 || 0 == strncmp(str1, str2, count); } @@ -89,6 +91,24 @@ rcutils_set_error_state( { rcutils_error_state_t error_state; + if (NULL == error_string) { +#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS + RCUTILS_SAFE_FWRITE_TO_STDERR( + "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) + "] rcutils_set_error_state() given null pointer for error_string, error was not set\n"); +#endif + return; + } + + if (NULL == file) { +#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS + RCUTILS_SAFE_FWRITE_TO_STDERR( + "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) + "] rcutils_set_error_state() given null pointer for file string, error was not set\n"); +#endif + return; + } + __rcutils_copy_string(error_state.message, sizeof(error_state.message), error_string); __rcutils_copy_string(error_state.file, sizeof(error_state.file), file); error_state.line_number = line_number; @@ -152,6 +172,7 @@ rcutils_get_error_string(void) } if (!gtls_rcutils_error_string_is_formatted) { __rcutils_format_error_string(>ls_rcutils_error_string, >ls_rcutils_error_state); + gtls_rcutils_error_string_is_formatted = true; } return gtls_rcutils_error_string; } diff --git a/src/error_handling_helpers.h b/src/error_handling_helpers.h index 7a9bc2bf..542368ad 100644 --- a/src/error_handling_helpers.h +++ b/src/error_handling_helpers.h @@ -1,4 +1,4 @@ -// Copyright 2014 Open Source Robotics Foundation, Inc. +// Copyright 2018 Open Source Robotics Foundation, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -54,7 +54,7 @@ __rcutils_copy_string(char * dst, size_t dst_size, const char * src) assert(dst_size > 0); assert(src != NULL); // doesn't matter how long src actually is if it is longer than dst, so limit to dst + 1 - size_t src_length = strnlen(src, dst_size + 1); + size_t src_length = strnlen(src, dst_size); size_t size_to_copy = src_length; // the destination must be one byte bigger to store the NULL terminating character if (src_length >= dst_size) { @@ -91,7 +91,7 @@ void __rcutils_reverse_str(char * string_in, size_t string_len) { assert(string_in != NULL); - if (string_len == 0) { + if (0 == string_len) { return; } size_t i = 0; diff --git a/test/test_error_handling.cpp b/test/test_error_handling.cpp index 4ee9b42e..f0623bca 100644 --- a/test/test_error_handling.cpp +++ b/test/test_error_handling.cpp @@ -86,7 +86,9 @@ Stack trace (most recent call last): rcutils_reset_error(); }); const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message); + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(test_message); + }); using ::testing::StartsWith; EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); rcutils_error_string_t error_string = rcutils_get_error_string(); @@ -107,7 +109,9 @@ TEST(test_error_handling, reset) { }); { const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message); + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(test_message); + }); using ::testing::StartsWith; EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); rcutils_error_string_t error_string = rcutils_get_error_string(); @@ -117,7 +121,9 @@ TEST(test_error_handling, reset) { rcutils_reset_error(); { const char * test_message = "different message"; - RCUTILS_SET_ERROR_MSG(test_message); + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(test_message); + }); using ::testing::StartsWith; EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); rcutils_error_string_t error_string = rcutils_get_error_string(); @@ -144,6 +150,43 @@ TEST(test_error_handling, reset) { }); } +TEST(test_error_handling, invalid_arguments) { + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + rcutils_ret_t ret = + rcutils_initialize_error_handling_thread_local_storage(rcutils_get_default_allocator()); + ASSERT_EQ(ret, RCUTILS_RET_OK); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); + printf("The following error from within error_handling.c is expected.\n"); + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(NULL); + }); + EXPECT_FALSE(rcutils_error_is_set()); + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_STREQ("error not set", error_string.str); + } + + printf("The following error from within error_handling.c is expected.\n"); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_set_error_state("valid error message", NULL, 42); + }); + EXPECT_FALSE(rcutils_error_is_set()); + { + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_STREQ("error not set", error_string.str); + } + + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); +} + TEST(test_error_handling, empty) { osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; rcutils_ret_t ret = @@ -178,7 +221,9 @@ TEST(test_error_handling, recursive) { rcutils_reset_error(); }); const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message); + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(test_message); + }); using ::testing::HasSubstr; { EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); @@ -211,7 +256,9 @@ TEST(test_error_handling, copy) { rcutils_reset_error(); }); const char * test_message = "test message"; - RCUTILS_SET_ERROR_MSG(test_message); + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(test_message); + }); using ::testing::HasSubstr; EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); rcutils_error_string_t error_string = rcutils_get_error_string(); diff --git a/test/test_error_handling_helpers.cpp b/test/test_error_handling_helpers.cpp index 3e3a5c17..91ecd57d 100644 --- a/test/test_error_handling_helpers.cpp +++ b/test/test_error_handling_helpers.cpp @@ -1,4 +1,4 @@ -// Copyright 2016 Open Source Robotics Foundation, Inc. +// Copyright 2018 Open Source Robotics Foundation, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From dc725ecdc51b52e3d2bfb9bc3d7a838d5e18e5e5 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Thu, 1 Nov 2018 18:40:25 -0500 Subject: [PATCH 19/20] avoid fprintf and fix warning about overwriting Signed-off-by: William Woodall --- include/rcutils/error_handling.h | 12 ++-- src/error_handling.c | 106 +++++++++++++++++++++++++------ test/test_error_handling.cpp | 30 +++++++++ 3 files changed, 122 insertions(+), 26 deletions(-) diff --git a/include/rcutils/error_handling.h b/include/rcutils/error_handling.h index 6e05e93a..610e4f1b 100644 --- a/include/rcutils/error_handling.h +++ b/include/rcutils/error_handling.h @@ -61,12 +61,12 @@ extern "C" // e.g. "some error, at /path/to/a.c:42, at /path/to/b.c:42" #define RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH 768 // with RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH = 768, RCUTILS_ERROR_STATE_FILE_MAX_LENGTH == 229 -#define RCUTILS_ERROR_STATE_FILE_MAX_LENGTH (\ - RCUTILS_ERROR_MESSAGE_MAX_LENGTH - \ - RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH - \ - RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH - \ - RCUTILS_ERROR_FORMATTING_CHARACTERS - \ - 1) +#define RCUTILS_ERROR_STATE_FILE_MAX_LENGTH ( \ + RCUTILS_ERROR_MESSAGE_MAX_LENGTH - \ + RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH - \ + RCUTILS_ERROR_STATE_LINE_NUMBER_STR_MAX_LENGTH - \ + RCUTILS_ERROR_FORMATTING_CHARACTERS - \ + 1) /// Struct wrapping a fixed-size c string used for returning the formatted error string. typedef struct rcutils_error_string_t diff --git a/src/error_handling.c b/src/error_handling.c index 74eb2bbb..49bf2cfa 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -83,6 +83,87 @@ __same_string(const char * str1, const char * str2, size_t count) return str1 == str2 || 0 == strncmp(str1, str2, count); } +static +void +__format_overwriting_error_state_message( + char * buffer, + size_t buffer_size, + const rcutils_error_state_t * new_error_state) +{ + assert(NULL != buffer); + assert(0 != buffer_size); + assert(INT64_MAX > buffer_size); + assert(NULL != new_error_state); + + int64_t bytes_left = buffer_size; + do { + char * offset = buffer; + size_t written = 0; + + // write the first static part of the error message + written = __rcutils_copy_string(offset, bytes_left, + "\n" + ">>> [rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) "] rcutils_set_error_state()\n" + "This error state is being overwritten:\n" + "\n" + " '"); + offset += written; + bytes_left -= written; + if (0 >= bytes_left) {break;} + + // write the old error string + rcutils_error_string_t old_error_string = rcutils_get_error_string(); + written = __rcutils_copy_string(offset, sizeof(old_error_string.str), old_error_string.str); + offset += written; + bytes_left -= written; + if (0 >= bytes_left) {break;} + + // write the middle part of the state error message + written = __rcutils_copy_string(offset, bytes_left, + "'\n" + "\n" + "with this new error message:\n" + "\n" + " '"); + offset += written; + bytes_left -= written; + if (0 >= bytes_left) {break;} + + // format error string for new error state and write it in + rcutils_error_string_t new_error_string = { + .str = "\0" + }; + __rcutils_format_error_string(&new_error_string, new_error_state); + written = __rcutils_copy_string(offset, sizeof(new_error_string.str), new_error_string.str); + offset += written; + bytes_left -= written; + if (0 >= bytes_left) {break;} + + // write the last part of the state error message + written = __rcutils_copy_string(offset, bytes_left, + "'\n" + "\n" + "rcutils_reset_error() should be called after error handling to avoid this.\n" + "<<<\n"); + bytes_left -= written; + if (bytes_left <= 0) {break;} + } while (0); + +#if RCUTILS_REPORT_ERROR_HANDLING_ERRORS + // note that due to the way truncating is done above, and the size of the + // output buffer used with this function in rcutils_set_error_state() below, + // this should never evaluate to true, but it's here to be defensive and try + // to catch programming mistakes in this file earlier. + if (0 >= bytes_left) { + RCUTILS_SAFE_FWRITE_TO_STDERR( + "[rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) + "] rcutils_set_error_state() following error message was too long and will be truncated\n"); + } +#else + (void)bytes_left; // avoid scope could be reduced warning if in this case +#endif +} + void rcutils_set_error_state( const char * error_string, @@ -121,27 +202,12 @@ rcutils_set_error_state( "expected error state's max message length to be less than or equal to error string max"); if ( gtls_rcutils_error_is_set && - __same_string(error_string, gtls_rcutils_error_string.str, characters_to_compare) && - __same_string(error_string, gtls_rcutils_error_state.message, characters_to_compare)) + !__same_string(error_string, gtls_rcutils_error_string.str, characters_to_compare) && + !__same_string(error_string, gtls_rcutils_error_state.message, characters_to_compare)) { - fprintf( - stderr, - "\n" - ">>> [rcutils|error_handling.c:" RCUTILS_STRINGIFY(__LINE__) "] rcutils_set_error_state()\n" - "This error state is being overwritten:\n" - "\n" - " '%s'\n" - "\n" - "with this new error message:\n" - "\n" - " '%s, at %s:%zu'\n" - "\n" - "rcutils_reset_error() should be called after error handling to avoid this.\n" - "<<<\n", - gtls_rcutils_error_string.str, - error_string, - file, - line_number); + char output_buffer[4096]; + __format_overwriting_error_state_message(output_buffer, sizeof(output_buffer), &error_state); + RCUTILS_SAFE_FWRITE_TO_STDERR(output_buffer); } #endif gtls_rcutils_error_state = error_state; diff --git a/test/test_error_handling.cpp b/test/test_error_handling.cpp index f0623bca..883d0ae5 100644 --- a/test/test_error_handling.cpp +++ b/test/test_error_handling.cpp @@ -280,3 +280,33 @@ TEST(test_error_handling, copy) { rcutils_reset_error(); }); } + +TEST(test_error_handling, overwrite) { + osrf_testing_tools_cpp::memory_tools::ScopedQuickstartGtest scoped_quickstart_gtest; + rcutils_ret_t ret = + rcutils_initialize_error_handling_thread_local_storage(rcutils_get_default_allocator()); + ASSERT_EQ(ret, RCUTILS_RET_OK); + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); + const char * test_message = "this is expected to cause a warning from error_handling.c"; + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(test_message); + }); + using ::testing::HasSubstr; + EXPECT_NO_MEMORY_OPERATIONS_BEGIN(); + rcutils_error_string_t error_string = rcutils_get_error_string(); + EXPECT_NO_MEMORY_OPERATIONS_END(); + ASSERT_THAT(error_string.str, HasSubstr(", at")); + + // force an overwrite error + const char * test_message2 = "and this too"; + printf("The following warning from error_handling.c is expected...\n"); + EXPECT_NO_MEMORY_OPERATIONS({ + RCUTILS_SET_ERROR_MSG(test_message2); + }); + + EXPECT_NO_MEMORY_OPERATIONS({ + rcutils_reset_error(); + }); +} From f81a5c8f14a62a30cc0dce5cd3eb8fac90b55178 Mon Sep 17 00:00:00 2001 From: William Woodall Date: Thu, 1 Nov 2018 19:58:42 -0500 Subject: [PATCH 20/20] remove unnecessary call --- src/error_handling.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/error_handling.c b/src/error_handling.c index 49bf2cfa..01f34588 100644 --- a/src/error_handling.c +++ b/src/error_handling.c @@ -146,7 +146,6 @@ __format_overwriting_error_state_message( "rcutils_reset_error() should be called after error handling to avoid this.\n" "<<<\n"); bytes_left -= written; - if (bytes_left <= 0) {break;} } while (0); #if RCUTILS_REPORT_ERROR_HANDLING_ERRORS