From 4beed347e7cb7e41ff7d9765c4c11da74248c946 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 8 Jul 2020 18:07:43 -0300 Subject: [PATCH 1/3] Add needed extra null check Signed-off-by: Jorge Perez --- rcl/src/rcl/remap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/rcl/src/rcl/remap.c b/rcl/src/rcl/remap.c index 5a84a636a..950ff9761 100644 --- a/rcl/src/rcl/remap.c +++ b/rcl/src/rcl/remap.c @@ -43,6 +43,7 @@ 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_CHECK_ARGUMENT_FOR_NULL(rule->impl, RCL_RET_INVALID_ARGUMENT); if (NULL != rule_out->impl) { RCL_SET_ERROR_MSG("rule_out must be zero initialized"); From 2882bc05bcc2568fc91d92ab7b2585cfab601a70 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 16 Jul 2020 19:21:24 -0300 Subject: [PATCH 2/3] Add test for added check Signed-off-by: Jorge Perez --- rcl/test/rcl/test_remap.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/rcl/test/rcl/test_remap.cpp b/rcl/test/rcl/test_remap.cpp index f2dca9dbe..7c4888592 100644 --- a/rcl/test/rcl/test_remap.cpp +++ b/rcl/test/rcl/test_remap.cpp @@ -596,6 +596,11 @@ TEST_F(CLASSNAME(TestRemapFixture, RMW_IMPLEMENTATION), internal_remap_use) { EXPECT_EQ(RCL_RET_BAD_ALLOC, rcl_remap_copy(parsed_args.impl->remap_rules, &remap_dst)); parsed_args.impl->remap_rules->impl->allocator = alloc; + // Not valid empty source + rcl_remap_t remap_empty = rcl_get_zero_initialized_remap(); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remap_copy(&remap_empty, &remap_dst)); + rcl_reset_error(); + // Expected usage EXPECT_EQ(RCL_RET_OK, rcl_remap_copy(parsed_args.impl->remap_rules, &remap_dst)); From 4271d7f75a467f7d648c2dbf3f4e97f28685e5ee Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 16 Jul 2020 19:30:24 -0300 Subject: [PATCH 3/3] Add tests for nullptrs Signed-off-by: Jorge Perez --- rcl/test/rcl/test_remap.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rcl/test/rcl/test_remap.cpp b/rcl/test/rcl/test_remap.cpp index 7c4888592..72016d867 100644 --- a/rcl/test/rcl/test_remap.cpp +++ b/rcl/test/rcl/test_remap.cpp @@ -596,6 +596,12 @@ TEST_F(CLASSNAME(TestRemapFixture, RMW_IMPLEMENTATION), internal_remap_use) { EXPECT_EQ(RCL_RET_BAD_ALLOC, rcl_remap_copy(parsed_args.impl->remap_rules, &remap_dst)); parsed_args.impl->remap_rules->impl->allocator = alloc; + // Not valid null ptrs + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remap_copy(nullptr, &remap_dst)); + rcl_reset_error(); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remap_copy(parsed_args.impl->remap_rules, nullptr)); + rcl_reset_error(); + // Not valid empty source rcl_remap_t remap_empty = rcl_get_zero_initialized_remap(); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remap_copy(&remap_empty, &remap_dst));