From 06d57452987880bfb4ee78df046a13152ffcd448 Mon Sep 17 00:00:00 2001
From: Chris Lalancette <clalancette@gmail.com>
Date: Mon, 28 Aug 2023 09:17:42 -0400
Subject: [PATCH] Simplify local_namespace handling in rcl_node_init. (#1097)

There is a comment in there about avoiding a memory allocation,
but that doesn't seem to make much sense to me; we are
going to be doing an allocation directly below, so avoiding
it here seems unnecessary and makes the code harder
to follow.  Simplify everything by always making an allocation

Signed-off-by: Chris Lalancette <clalancette@gmail.com>
---
 rcl/src/rcl/node.c | 50 ++++++++++++++++------------------------------
 1 file changed, 17 insertions(+), 33 deletions(-)

diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c
index b0fe0cfc2..dc71eb6b0 100644
--- a/rcl/src/rcl/node.c
+++ b/rcl/src/rcl/node.c
@@ -12,11 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#ifdef __cplusplus
-extern "C"
-{
-#endif
-
 #include "rcl/node.h"
 
 #include <limits.h>
@@ -162,24 +157,21 @@ rcl_node_init(
   }
 
   // Process the namespace.
-  size_t namespace_length = strlen(namespace_);
-  const char * local_namespace_ = namespace_;
-  bool should_free_local_namespace_ = false;
-  // If the namespace is just an empty string, replace with "/"
-  if (namespace_length == 0) {
-    // Have this special case to avoid a memory allocation when "" is passed.
-    local_namespace_ = "/";
-  }
-
-  // If the namespace does not start with a /, add one.
-  if (namespace_length > 0 && namespace_[0] != '/') {
+  const char * local_namespace_ = NULL;
+  if (namespace_[0] == '\0') {
+    // If the namespace is just an empty string, replace with "/"
+    local_namespace_ = rcutils_strdup("/", *allocator);
+  } else if (namespace_[0] == '/') {
+    local_namespace_ = rcutils_strdup(namespace_, *allocator);
+  } else {
+    // If the namespace does not start with a /, add one.
     local_namespace_ = rcutils_format_string(*allocator, "/%s", namespace_);
-    RCL_CHECK_FOR_NULL_WITH_MSG(
-      local_namespace_,
-      "failed to format node namespace string",
-      ret = RCL_RET_BAD_ALLOC; goto cleanup);
-    should_free_local_namespace_ = true;
   }
+  RCL_CHECK_FOR_NULL_WITH_MSG(
+    local_namespace_,
+    "failed to format node namespace string",
+    return RCL_RET_BAD_ALLOC);
+
   // Make sure the node namespace is valid.
   validation_result = 0;
   ret = rmw_validate_namespace(local_namespace_, &validation_result, NULL);
@@ -233,10 +225,7 @@ rcl_node_init(
   if (RCL_RET_OK != ret) {
     goto fail;
   } else if (NULL != remapped_namespace) {
-    if (should_free_local_namespace_) {
-      allocator->deallocate((char *)local_namespace_, allocator->state);
-    }
-    should_free_local_namespace_ = true;
+    allocator->deallocate((char *)local_namespace_, allocator->state);
     local_namespace_ = remapped_namespace;
   }
 
@@ -365,10 +354,9 @@ rcl_node_init(
   ret = fail_ret;
   // fall through from fail -> cleanup
 cleanup:
-  if (should_free_local_namespace_) {
-    allocator->deallocate((char *)local_namespace_, allocator->state);
-    local_namespace_ = NULL;
-  }
+  allocator->deallocate((char *)local_namespace_, allocator->state);
+  local_namespace_ = NULL;
+
   if (NULL != remapped_node_name) {
     allocator->deallocate(remapped_node_name, allocator->state);
   }
@@ -693,7 +681,3 @@ rcl_ret_t rcl_node_get_type_description_service(
   *service_out = &node->impl->get_type_description_service;
   return RCL_RET_OK;
 }
-
-#ifdef __cplusplus
-}
-#endif