Make rcl_remap_t use the PIMPL pattern (#377)

Signed-off-by: Michael Carroll <carroll.michael@gmail.com>

rcl/include/rcl/remap.h

@@ -26,6 +26,21 @@ extern "C"
 {
 #endif
 
+struct rcl_remap_impl_t;
+
+/// Hold remapping rules.
+typedef struct rcl_remap_t
+{
+  /// Private implementation pointer.
+  struct rcl_remap_impl_t * impl;
+} rcl_remap_t;
+
+/// Return a rcl_remap_t struct with members initialized to `NULL`.
+RCL_PUBLIC
+RCL_WARN_UNUSED
+rcl_remap_t
+rcl_get_zero_initialized_remap(void);
+
 // TODO(sloretz) add documentation about rostopic:// when it is supported
 /// Remap a topic name based on given rules.
 /**
@@ -232,6 +247,27 @@ rcl_remap_node_namespace(
   rcl_allocator_t allocator,
   char ** output_namespace);
 
+/// Reclaim resources held inside rcl_remap_t structure.
+/**
+ * <hr>
+ * Attribute          | Adherence
+ * ------------------ | -------------
+ * Allocates Memory   | No
+ * Thread-Safe        | Yes
+ * Uses Atomics       | No
+ * Lock-Free          | Yes
+ *
+ * \param[in] args The structure to be deallocated.
+ * \return `RCL_RET_OK` if the memory was successfully freed, or
+ * \return `RCL_RET_INVALID_ARGUMENT` if any function arguments are invalid, or
+ * \return `RCL_RET_ERROR` if an unspecified error occurs.
+ */
+RCL_PUBLIC
+RCL_WARN_UNUSED
+rcl_ret_t
+rcl_remap_fini(
+  rcl_remap_t * remap);
+
 #ifdef __cplusplus
 }
 #endif

rcl/src/rcl/arguments.c

@@ -267,7 +267,7 @@ rcl_parse_arguments(
 
     // Attempt to parse argument as remap rule
     rcl_remap_t * rule = &(args_impl->remap_rules[args_impl->num_remap_rules]);
-    *rule = rcl_remap_get_zero_initialized();
+    *rule = rcl_get_zero_initialized_remap();
     if (RCL_RET_OK == _rcl_parse_remap_rule(argv[i], allocator, rule)) {
       ++(args_impl->num_remap_rules);
       continue;
@@ -533,7 +533,7 @@ rcl_arguments_copy(
     }
     args_out->impl->num_remap_rules = args->impl->num_remap_rules;
     for (int i = 0; i < args->impl->num_remap_rules; ++i) {
-      args_out->impl->remap_rules[i] = rcl_remap_get_zero_initialized();
+      args_out->impl->remap_rules[i] = rcl_get_zero_initialized_remap();
       rcl_ret_t ret = rcl_remap_copy(
         &(args->impl->remap_rules[i]), &(args_out->impl->remap_rules[i]));
       if (RCL_RET_OK != ret) {
@@ -737,8 +737,9 @@ _rcl_parse_remap_replacement_name(
   // Copy replacement into rule
   const char * replacement_end = rcl_lexer_lookahead2_get_text(lex_lookahead);
   size_t length = (size_t)(replacement_end - replacement_start);
-  rule->replacement = rcutils_strndup(replacement_start, length, rule->allocator);
-  if (NULL == rule->replacement) {
+  rule->impl->replacement = rcutils_strndup(
+    replacement_start, length, rule->impl->allocator);
+  if (NULL == rule->impl->replacement) {
     RCL_SET_ERROR_MSG("failed to copy replacement");
     return RCL_RET_BAD_ALLOC;
   }
@@ -796,13 +797,13 @@ _rcl_parse_remap_match_name(
     return ret;
   }
   if (RCL_LEXEME_URL_SERVICE == lexeme) {
-    rule->type = RCL_SERVICE_REMAP;
+    rule->impl->type = RCL_SERVICE_REMAP;
     ret = rcl_lexer_lookahead2_accept(lex_lookahead, NULL, NULL);
   } else if (RCL_LEXEME_URL_TOPIC == lexeme) {
-    rule->type = RCL_TOPIC_REMAP;
+    rule->impl->type = RCL_TOPIC_REMAP;
     ret = rcl_lexer_lookahead2_accept(lex_lookahead, NULL, NULL);
   } else {
-    rule->type = (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP);
+    rule->impl->type = (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP);
   }
   if (RCL_RET_OK != ret) {
     return ret;
@@ -853,8 +854,8 @@ _rcl_parse_remap_match_name(
   // Copy match into rule
   const char * match_end = rcl_lexer_lookahead2_get_text(lex_lookahead);
   size_t length = (size_t)(match_end - match_start);
-  rule->match = rcutils_strndup(match_start, length, rule->allocator);
-  if (NULL == rule->match) {
+  rule->impl->match = rcutils_strndup(match_start, length, rule->impl->allocator);
+  if (NULL == rule->impl->match) {
     RCL_SET_ERROR_MSG("failed to copy match");
     return RCL_RET_BAD_ALLOC;
   }
@@ -940,13 +941,13 @@ _rcl_parse_remap_namespace_replacement(
   // Copy namespace into rule
   const char * ns_end = rcl_lexer_lookahead2_get_text(lex_lookahead);
   size_t length = (size_t)(ns_end - ns_start);
-  rule->replacement = rcutils_strndup(ns_start, length, rule->allocator);
-  if (NULL == rule->replacement) {
+  rule->impl->replacement = rcutils_strndup(ns_start, length, rule->impl->allocator);
+  if (NULL == rule->impl->replacement) {
     RCL_SET_ERROR_MSG("failed to copy namespace");
     return RCL_RET_BAD_ALLOC;
   }
 
-  rule->type = RCL_NAMESPACE_REMAP;
+  rule->impl->type = RCL_NAMESPACE_REMAP;
   return RCL_RET_OK;
 }
 
@@ -983,13 +984,13 @@ _rcl_parse_remap_nodename_replacement(
     return ret;
   }
   // copy the node name into the replacement side of the rule
-  rule->replacement = rcutils_strndup(node_name, length, rule->allocator);
-  if (NULL == rule->replacement) {
+  rule->impl->replacement = rcutils_strndup(node_name, length, rule->impl->allocator);
+  if (NULL == rule->impl->replacement) {
     RCL_SET_ERROR_MSG("failed to allocate node name");
     return RCL_RET_BAD_ALLOC;
   }
 
-  rule->type = RCL_NODENAME_REMAP;
+  rule->impl->type = RCL_NODENAME_REMAP;
   return RCL_RET_OK;
 }
 
@@ -1018,8 +1019,8 @@ _rcl_parse_remap_nodename_prefix(
   }
 
   // copy the node name into the rule
-  rule->node_name = rcutils_strndup(node_name, length, rule->allocator);
-  if (NULL == rule->node_name) {
+  rule->impl->node_name = rcutils_strndup(node_name, length, rule->impl->allocator);
+  if (NULL == rule->impl->node_name) {
     RCL_SET_ERROR_MSG("failed to allocate node name");
     return RCL_RET_BAD_ALLOC;
   }
@@ -1123,7 +1124,16 @@ _rcl_parse_remap_rule(
 
   rcl_ret_t ret;
 
-  output_rule->allocator = allocator;
+  output_rule->impl = allocator.allocate(sizeof(rcl_remap_impl_t), allocator.state);
+  if (NULL == output_rule->impl) {
+    return RCL_RET_BAD_ALLOC;
+  }
+  output_rule->impl->allocator = allocator;
+  output_rule->impl->type = RCL_UNKNOWN_REMAP;
+  output_rule->impl->node_name = NULL;
+  output_rule->impl->match = NULL;
+  output_rule->impl->replacement = NULL;
+
   rcl_lexer_lookahead2_t lex_lookahead = rcl_get_zero_initialized_lexer_lookahead2();
 
   ret = rcl_lexer_lookahead2_init(&lex_lookahead, arg, allocator);

rcl/src/rcl/remap.c

@@ -28,15 +28,12 @@ extern "C"
 #endif
 
 rcl_remap_t
-rcl_remap_get_zero_initialized()
+rcl_get_zero_initialized_remap(void)
 {
-  rcl_remap_t rule;
-  rule.type = RCL_UNKNOWN_REMAP;
-  rule.node_name = NULL;
-  rule.match = NULL;
-  rule.replacement = NULL;
-  rule.allocator = rcutils_get_zero_initialized_allocator();
-  return rule;
+  static rcl_remap_t default_rule = {
+    .impl = NULL
+  };
+  return default_rule;
 }
 
 rcl_ret_t
@@ -47,24 +44,43 @@ rcl_remap_copy(
   RCL_CHECK_ARGUMENT_FOR_NULL(rule, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(rule_out, RCL_RET_INVALID_ARGUMENT);
 
-  rcl_allocator_t allocator = rule->allocator;
-  rule_out->allocator = allocator;
-  rule_out->type = rule->type;
-  if (NULL != rule->node_name) {
-    rule_out->node_name = rcutils_strdup(rule->node_name, allocator);
-    if (NULL == rule_out->node_name) {
+  if (NULL != rule_out->impl) {
+    RCL_SET_ERROR_MSG("rule_out must be zero initialized");
+    return RCL_RET_INVALID_ARGUMENT;
+  }
+
+  rcl_allocator_t allocator = rule->impl->allocator;
+
+  rule_out->impl = allocator.allocate(sizeof(rcl_remap_impl_t), allocator.state);
+  if (NULL == rule_out->impl) {
+    return RCL_RET_BAD_ALLOC;
+  }
+
+  rule_out->impl->allocator = allocator;
+
+  // Zero so it's safe to call rcl_remap_fini() if an error occurs while copying.
+  rule_out->impl->type = RCL_UNKNOWN_REMAP;
+  rule_out->impl->node_name = NULL;
+  rule_out->impl->match = NULL;
+  rule_out->impl->replacement = NULL;
+
+  rule_out->impl->type = rule->impl->type;
+  if (NULL != rule->impl->node_name) {
+    rule_out->impl->node_name = rcutils_strdup(rule->impl->node_name, allocator);
+    if (NULL == rule_out->impl->node_name) {
       goto fail;
     }
   }
-  if (NULL != rule->match) {
-    rule_out->match = rcutils_strdup(rule->match, allocator);
-    if (NULL == rule_out->match) {
+  if (NULL != rule->impl->match) {
+    rule_out->impl->match = rcutils_strdup(rule->impl->match, allocator);
+    if (NULL == rule_out->impl->match) {
       goto fail;
     }
   }
-  if (NULL != rule->replacement) {
-    rule_out->replacement = rcutils_strdup(rule->replacement, allocator);
-    if (NULL == rule_out->replacement) {
+  if (NULL != rule->impl->replacement) {
+    rule_out->impl->replacement = rcutils_strdup(
+      rule->impl->replacement, allocator);
+    if (NULL == rule_out->impl->replacement) {
       goto fail;
     }
   }
@@ -76,26 +92,6 @@ rcl_remap_copy(
   return RCL_RET_BAD_ALLOC;
 }
 
-rcl_ret_t
-rcl_remap_fini(
-  rcl_remap_t * rule)
-{
-  if (NULL != rule->node_name) {
-    rule->allocator.deallocate(rule->node_name, rule->allocator.state);
-    rule->node_name = NULL;
-  }
-  if (NULL != rule->match) {
-    rule->allocator.deallocate(rule->match, rule->allocator.state);
-    rule->match = NULL;
-  }
-  if (NULL != rule->replacement) {
-    rule->allocator.deallocate(rule->replacement, rule->allocator.state);
-    rule->replacement = NULL;
-  }
-  rule->allocator = rcutils_get_zero_initialized_allocator();
-  return RCL_RET_OK;
-}
-
 /// Get the first matching rule in a chain.
 /// \return RCL_RET_OK if no errors occurred while searching for a rule
 RCL_LOCAL
@@ -114,20 +110,21 @@ _rcl_remap_first_match(
   *output_rule = NULL;
   for (int i = 0; i < num_rules; ++i) {
     rcl_remap_t * rule = &(remap_rules[i]);
-    if (!(rule->type & type_bitmask)) {
+    if (!(rule->impl->type & type_bitmask)) {
       // Not the type of remap rule we're looking fore
       continue;
     }
-    if (rule->node_name != NULL && 0 != strcmp(rule->node_name, node_name)) {
+    if (rule->impl->node_name != NULL && 0 != strcmp(rule->impl->node_name, node_name)) {
       // Rule has a node name prefix and the supplied node name didn't match
       continue;
     }
     bool matched = false;
-    if (rule->type & (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP)) {
+    if (rule->impl->type & (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP)) {
       // topic and service rules need the match side to be expanded to a FQN
       char * expanded_match = NULL;
       rcl_ret_t ret = rcl_expand_topic_name(
-        rule->match, node_name, node_namespace, substitutions, allocator, &expanded_match);
+        rule->impl->match, node_name, node_namespace,
+        substitutions, allocator, &expanded_match);
       if (RCL_RET_OK != ret) {
         rcl_reset_error();
         if (
@@ -204,16 +201,16 @@ _rcl_remap_name(
   }
   // Do the remapping
   if (NULL != rule) {
-    if (rule->type & (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP)) {
+    if (rule->impl->type & (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP)) {
       // topic and service rules need the replacement to be expanded to a FQN
       rcl_ret_t ret = rcl_expand_topic_name(
-        rule->replacement, node_name, node_namespace, substitutions, allocator, output_name);
+        rule->impl->replacement, node_name, node_namespace, substitutions, allocator, output_name);
       if (RCL_RET_OK != ret) {
         return ret;
       }
     } else {
       // nodename and namespace rules don't need replacment expanded
-      *output_name = rcutils_strdup(rule->replacement, allocator);
+      *output_name = rcutils_strdup(rule->impl->replacement, allocator);
     }
     if (NULL == *output_name) {
       RCL_SET_ERROR_MSG("Failed to set output");
@@ -311,6 +308,36 @@ rcl_remap_node_namespace(
     allocator, output_namespace);
 }
 
+rcl_ret_t
+rcl_remap_fini(
+  rcl_remap_t * rule)
+{
+  RCL_CHECK_ARGUMENT_FOR_NULL(rule, RCL_RET_INVALID_ARGUMENT);
+  if (rule->impl) {
+    rcl_ret_t ret = RCL_RET_OK;
+    if (NULL != rule->impl->node_name) {
+      rule->impl->allocator.deallocate(
+        rule->impl->node_name, rule->impl->allocator.state);
+      rule->impl->node_name = NULL;
+    }
+    if (NULL != rule->impl->match) {
+      rule->impl->allocator.deallocate(
+        rule->impl->match, rule->impl->allocator.state);
+      rule->impl->match = NULL;
+    }
+    if (NULL != rule->impl->replacement) {
+      rule->impl->allocator.deallocate(
+        rule->impl->replacement, rule->impl->allocator.state);
+      rule->impl->replacement = NULL;
+    }
+    rule->impl->allocator.deallocate(rule->impl, rule->impl->allocator.state);
+    rule->impl = NULL;
+    return ret;
+  }
+  RCL_SET_ERROR_MSG("rcl_remap_t finalized twice");
+  return RCL_RET_ERROR;
+}
+
 #ifdef __cplusplus
 }
 #endif

rcl/src/rcl/remap_impl.h

@@ -17,6 +17,7 @@
 
 #include "rcl/allocator.h"
 #include "rcl/macros.h"
+#include "rcl/remap.h"
 #include "rcl/types.h"
 #include "rcl/visibility_control.h"
 
@@ -35,7 +36,7 @@ typedef enum rcl_remap_type_t
   RCL_NAMESPACE_REMAP = 1u << 3
 } rcl_remap_type_t;
 
-typedef struct rcl_remap_t
+typedef struct rcl_remap_impl_t
 {
   /// Bitmask indicating what type of rule this is.
   rcl_remap_type_t type;
@@ -48,7 +49,7 @@ typedef struct rcl_remap_t
 
   /// Allocator used to allocate objects in this struct
   rcl_allocator_t allocator;
-} rcl_remap_t;
+} rcl_remap_impl_t;
 
 /// Get an rcl_remap_t structure initialized with NULL.
 rcl_remap_t