memory leak issues (#63)

* * memory leak issues
memory allocated from rcutils_format_string() is relatively easy
easy to forget to free and this leaks system resource. A wrapper
API added to make it shared and centralized

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>

* * Define a helper MACRO and fix memory leak with it
define the macro RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING
to help format error string and fix those memory leak issues
with this marco

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>

* fixup documentation and variable names

* adjust header file and add marco namespace

* avoid unknown free to invalid address

If the `goto fail;` at a certain allocation failure in the middle, just to
free those memory before it in `rcutils_string_array_fini()`, those
after that failure is invalid

* tweak marco definition to be safer while using

* minor grammar fix

* write out error in rcutils_string_array_fini

* typo

* missing continuation in macro

* uncrustify

* remove unused variable

* while (0) -> while (false) reapplied from 536ae0e

include/rcutils/error_handling.h

@@ -24,6 +24,8 @@ extern "C"
 
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
 
 #include "rcutils/allocator.h"
 #include "rcutils/macros.h"
@@ -39,6 +41,8 @@ typedef struct rcutils_error_state_t
   rcutils_allocator_t allocator;
 } rcutils_error_state_t;
 
+#define RCUTILS_SAFE_FWRITE_TO_STDERR(msg) fwrite(msg, sizeof(char), sizeof(msg), stderr)
+
 /// Copy an error state into a destination error state.
 /**
  * The destination state must be empty, the memory will not be free'd.
@@ -132,6 +136,27 @@ rcutils_set_error_state(
 #define RCUTILS_SET_ERROR_MSG(msg, allocator) \
   rcutils_set_error_state(msg, __FILE__, __LINE__, allocator);
 
+/// 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.
+ *
+ * \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, ...) \
+  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); \
+    } else { \
+      RCUTILS_SAFE_FWRITE_TO_STDERR("Failed to allocate memory for error message\n"); \
+    } \
+  } while (false)
+
 /// Return `true` if the error is set, otherwise `false`.
 RCUTILS_PUBLIC
 bool

src/split.c

@@ -58,7 +58,6 @@ rcutils_split(
     rhs_offset = 1;
   }
 
-  const char * error_msg;
   string_array->size = 1;
   for (size_t i = lhs_offset; i < string_size - rhs_offset; ++i) {
     if (str[i] == delimiter) {
@@ -87,6 +86,7 @@ rcutils_split(
         string_array->data[token_counter] =
           allocator.allocate((rhs - lhs + 2) * sizeof(char), allocator.state);
         if (!string_array->data[token_counter]) {
+          string_array->size = token_counter;
           goto fail;
         }
         snprintf(string_array->data[token_counter], (rhs - lhs + 1), "%s", str + lhs);
@@ -111,11 +111,14 @@ rcutils_split(
   return RCUTILS_RET_OK;
 
 fail:
-  error_msg = "unable to allocate memory for string array data";
   if (rcutils_string_array_fini(string_array) != RCUTILS_RET_OK) {
-    error_msg = rcutils_format_string(allocator, "FATAL: %s. Leaking memory", error_msg);
+    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("\n");
+    rcutils_reset_error();
   }
-  RCUTILS_SET_ERROR_MSG(error_msg, allocator);
+
+  RCUTILS_SET_ERROR_MSG("unable to allocate memory for string array data", allocator);
   return RCUTILS_RET_ERROR;
 }
 

src/string_map.c

@@ -353,9 +353,7 @@ rcutils_string_map_unset(rcutils_string_map_t * string_map, const char * key)
   rcutils_allocator_t allocator = string_map->impl->allocator;
   size_t key_index;
   if (!__get_index_of_key_if_exists(string_map->impl, key, strlen(key), &key_index)) {
-    char * msg = rcutils_format_string(allocator, "key '%s' not found", key);
-    RCUTILS_SET_ERROR_MSG(msg, allocator)
-    allocator.deallocate(msg, allocator.state);
+    RCUTILS_SET_ERROR_MSG_WITH_FORMAT_STRING(allocator, "key '%s' not found", key);
     return RCUTILS_RET_STRING_KEY_NOT_FOUND;
   }
   __remove_key_and_value_at_index(string_map->impl, key_index);