Skip to content

Commit

Permalink
Add check rcl_node_options_copy invalid out (#671)
Browse files Browse the repository at this point in the history
* 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 <jjperez@ekumenlabs.com>
  • Loading branch information
Blast545 authored and jacobperron committed Jun 24, 2020
1 parent d663b4b commit 0e12a0a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 6 deletions.
4 changes: 4 additions & 0 deletions rcl/src/rcl/node_options.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 13 additions & 6 deletions rcl/test/rcl/test_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(&not_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, &not_ini_options));
// EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&not_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));
}

0 comments on commit 0e12a0a

Please sign in to comment.