From 0e12a0a19b83a4a822ab74a78e3d92750453c183 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Fri, 19 Jun 2020 17:55:47 -0300 Subject: [PATCH] Add check rcl_node_options_copy invalid out (#671) * Add bug fix for rcl_node_options_copy If copy source has options->arguments.impl==NULL this will cause problem when calling fini on options dst * Add test case for bug fix * Return error when passing not init object * Remove unexpected usage test * Init options with non empty arguments Signed-off-by: Jorge Perez --- rcl/src/rcl/node_options.c | 4 ++++ rcl/test/rcl/test_node.cpp | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/rcl/src/rcl/node_options.c b/rcl/src/rcl/node_options.c index 024a76dc6..77bedeff0 100644 --- a/rcl/src/rcl/node_options.c +++ b/rcl/src/rcl/node_options.c @@ -50,6 +50,10 @@ rcl_node_options_copy( RCL_SET_ERROR_MSG("Attempted to copy options into itself"); return RCL_RET_INVALID_ARGUMENT; } + if (NULL != options_out->arguments.impl) { + RCL_SET_ERROR_MSG("Options out must be zero initialized"); + return RCL_RET_INVALID_ARGUMENT; + } options_out->domain_id = options->domain_id; options_out->allocator = options->allocator; options_out->use_global_arguments = options->use_global_arguments; diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index 88a54e064..7a0253692 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -784,11 +784,18 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) { EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_fini(nullptr)); EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&default_options)); EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(¬_ini_options)); +} - // (TODO: blast545) will be fixed with: https://github.com/ros2/rcl/pull/671 - // This causes the test suite to fail: - // rcl_node_options_t default_options = rcl_node_get_default_options(); - // rcl_node_options_t not_ini_options; - // EXPECT_EQ(RCL_RET_OK, rcl_node_options_copy(&default_options, ¬_ini_options)); - // EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(¬_ini_options)); <-- code fails, returns -11 +/* Tests special case node_options + */ +TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options_fail) { + rcl_node_options_t prev_ini_options = rcl_node_get_default_options(); + const char * argv[] = {"--ros-args"}; + int argc = sizeof(argv) / sizeof(const char *); + EXPECT_EQ( + RCL_RET_OK, + rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &prev_ini_options.arguments)); + + rcl_node_options_t default_options = rcl_node_get_default_options(); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(&default_options, &prev_ini_options)); }