From 622068b9e45df93b4758ba786c5ce96202764ca3 Mon Sep 17 00:00:00 2001 From: Nikolai Morin Date: Fri, 18 Dec 2020 14:23:16 +0100 Subject: [PATCH 1/4] Add NULL check in remap.c Signed-off-by: Nikolai Morin --- rcl/src/rcl/remap.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rcl/src/rcl/remap.c b/rcl/src/rcl/remap.c index 7191c48c7..65c91a526 100644 --- a/rcl/src/rcl/remap.c +++ b/rcl/src/rcl/remap.c @@ -142,7 +142,9 @@ rcl_remap_first_match( } continue; } - matched = (0 == strcmp(expanded_match, name)); + if (NULL != name) { + 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 From 3fed3b1fe72087ad674bec84f22f3227c3c6d7c9 Mon Sep 17 00:00:00 2001 From: Nikolai Morin Date: Fri, 18 Dec 2020 14:41:39 +0100 Subject: [PATCH 2/4] Add check in rcl_remap_name instead Signed-off-by: Nikolai Morin --- rcl/src/rcl/remap.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rcl/src/rcl/remap.c b/rcl/src/rcl/remap.c index 65c91a526..d147ac4bb 100644 --- a/rcl/src/rcl/remap.c +++ b/rcl/src/rcl/remap.c @@ -142,9 +142,7 @@ rcl_remap_first_match( } continue; } - if (NULL != name) { - matched = (0 == strcmp(expanded_match, name)); - } + 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 @@ -174,6 +172,9 @@ rcl_remap_name( { RCL_CHECK_ARGUMENT_FOR_NULL(node_name, RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(output_name, RCL_RET_INVALID_ARGUMENT); + if (type_bitmask & (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP)) { + RCL_CHECK_ARGUMENT_FOR_NULL(name, RCL_RET_INVALID_ARGUMENT); + } if (NULL != local_arguments && NULL == local_arguments->impl) { local_arguments = NULL; } From 2e926f928fef8259d64be68395d60c5dfecd2fbf Mon Sep 17 00:00:00 2001 From: Nikolai Morin Date: Fri, 18 Dec 2020 14:47:54 +0100 Subject: [PATCH 3/4] Revert "Add check in rcl_remap_name instead" This reverts commit 3fed3b1fe72087ad674bec84f22f3227c3c6d7c9. Signed-off-by: Nikolai Morin --- rcl/src/rcl/remap.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/rcl/src/rcl/remap.c b/rcl/src/rcl/remap.c index d147ac4bb..65c91a526 100644 --- a/rcl/src/rcl/remap.c +++ b/rcl/src/rcl/remap.c @@ -142,7 +142,9 @@ rcl_remap_first_match( } continue; } - matched = (0 == strcmp(expanded_match, name)); + if (NULL != name) { + 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 @@ -172,9 +174,6 @@ rcl_remap_name( { RCL_CHECK_ARGUMENT_FOR_NULL(node_name, RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(output_name, RCL_RET_INVALID_ARGUMENT); - if (type_bitmask & (RCL_TOPIC_REMAP | RCL_SERVICE_REMAP)) { - RCL_CHECK_ARGUMENT_FOR_NULL(name, RCL_RET_INVALID_ARGUMENT); - } if (NULL != local_arguments && NULL == local_arguments->impl) { local_arguments = NULL; } From fd3b96302c5f52b12e767c6b9ca1293d21456b14 Mon Sep 17 00:00:00 2001 From: Nikolai Morin Date: Fri, 18 Dec 2020 15:03:46 +0100 Subject: [PATCH 4/4] Add comment & test Signed-off-by: Nikolai Morin --- rcl/src/rcl/remap.c | 3 +++ rcl/test/rcl/test_remap.cpp | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/rcl/src/rcl/remap.c b/rcl/src/rcl/remap.c index 65c91a526..d767ddd55 100644 --- a/rcl/src/rcl/remap.c +++ b/rcl/src/rcl/remap.c @@ -143,6 +143,9 @@ rcl_remap_first_match( continue; } 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); diff --git a/rcl/test/rcl/test_remap.cpp b/rcl/test/rcl/test_remap.cpp index 72016d867..b8ae663b7 100644 --- 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;