avoid dynamic memory allocation during error handling

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 "$<TARGET_FILE_DIR:${PROJECT_NAME}>"
+    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 "$<TARGET_FILE_DIR:${PROJECT_NAME}>"
+    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

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; \
   }
 

include/rcutils/error_handling.h

@@ -22,58 +22,123 @@ extern "C"
 {
 #endif
 
+#ifndef __STDC_WANT_LIB_EXT1__
+#define __STDC_WANT_LIB_EXT1__ 1  // indicate we would like strnlen_s if available
+#endif
+#include <assert.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
 #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"
 
+#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) \
+  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)
+#endif
+
+// 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 == 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)
+
+/// Struct wrapping a fixed-size c string used for returning the formatted error string.
+typedef struct rcutils_error_string_t
+{
+  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
 {
-  const char * message;
-  const char * file;
-  size_t line_number;
-  rcutils_allocator_t allocator;
+  /// User message storage, limited to RCUTILS_ERROR_STATE_MESSAGE_MAX_LENGTH characters.
+  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.
+  char file[RCUTILS_ERROR_STATE_FILE_MAX_LENGTH];
+  /// Line number of error.
+  uint64_t line_number;
 } rcutils_error_state_t;
 
-// TODO(dhood): use __STDC_LIB_EXT1__ if/when supported in other implementations.
-#if defined(_WIN32)
-// 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)
-#else
-#define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) fwrite(msg, sizeof(char), strlen(msg), stderr)
+// make sure our math is right...
+#if __STDC_VERSION__ >= 201112L
+static_assert(
+  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
 
-/// Copy an error state into a destination error state.
+/// 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.
  *
- * The copied error_state should be finalized with rcutils_error_state_fini().
+ * This function may or may not allocate memory.
+ * 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, 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.
  *
- * \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.
+ * 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.
+ *
+ * In case an error is indicated by the return code, no error message will have
+ * 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`, 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 +147,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 +165,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,13 +178,14 @@ 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) \
-  if (NULL == value) { \
-    RCUTILS_SET_ERROR_MSG(msg, allocator); \
-    error_statement; \
-  }
+#define RCUTILS_CHECK_FOR_NULL_WITH_MSG(value, 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.
 /**
@@ -140,34 +196,33 @@ 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) \
+  do {rcutils_set_error_state(msg, __FILE__, __LINE__);} while (0)
 
 /// 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)
+  } while (0)
 
 /// Return `true` if the error is set, otherwise `false`.
 RCUTILS_PUBLIC
+RCUTILS_WARN_UNUSED
 bool
 rcutils_error_is_set(void);
 
@@ -181,32 +236,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 <file>:<line>`, 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 <file>:<line>` 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 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.
  */
 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

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(); \
     } \