Add NULL check in remap.c (#879)

* Add NULL check in remap.c Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai>
ros2 · Dec 18, 2020 · 2ee91d8 · 2ee91d8
1 parent 30464d1
commit 2ee91d8
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 1 deletion.
diff --git a/rcl/src/rcl/remap.c b/rcl/src/rcl/remap.c
@@ -142,7 +142,12 @@ rcl_remap_first_match(
         }
         continue;
       }
-      matched = (0 == strcmp(expanded_match, name));
+      if (NULL != name) {
+        // this check is to satisfy clang-tidy – name is always not null when type_bitmask is
+        // RCL_TOPIC_REMAP or RCL_SERVICE_REMAP. That is guaranteed because rcl_remap_first_match
+        // and rcl_remap_name are not public.
+        matched = (0 == strcmp(expanded_match, name));
+      }
       allocator.deallocate(expanded_match, allocator.state);
     } else {
       // nodename and namespace replacement apply if the type and node name prefix checks passed

diff --git a/rcl/test/rcl/test_remap.cpp b/rcl/test/rcl/test_remap.cpp
@@ -172,6 +172,27 @@ TEST_F(CLASSNAME(TestRemapFixture, RMW_IMPLEMENTATION), global_topic_name_replac
   }
 }
 
+TEST_F(CLASSNAME(TestRemapFixture, RMW_IMPLEMENTATION), topic_and_service_name_not_null) {
+  rcl_ret_t ret;
+  rcl_arguments_t global_arguments;
+  SCOPE_ARGS(global_arguments, "process_name", "--ros-args", "-r", "/bar/foo:=/foo/bar");
+
+  {
+    char * output = NULL;
+    ret = rcl_remap_service_name(
+      NULL, &global_arguments, NULL, "NodeName", "/", rcl_get_default_allocator(), &output);
+    EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
+    ASSERT_EQ(NULL, output);
+  }
+  {
+    char * output = NULL;
+    ret = rcl_remap_topic_name(
+      NULL, &global_arguments, NULL, "NodeName", "/", rcl_get_default_allocator(), &output);
+    EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
+    EXPECT_EQ(NULL, output);
+  }
+}
+
 TEST_F(CLASSNAME(TestRemapFixture, RMW_IMPLEMENTATION), relative_topic_name_remap) {
   rcl_ret_t ret;
   rcl_arguments_t global_arguments;