niklas/rcl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Add check rcl_node_options_copy invalid out (#671)

Browse source 
* 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>
This commit is contained in:
Jorge Perez 2020-06-19 17:55:47 -03:00 • committed by Jacob Perron
parent d663b4b9b7
commit 0e12a0a19b
2 changed files with 18 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

4
rcl/src/rcl/node_options.c
View file

@ -50,6 +50,10 @@ rcl_node_options_copy(
RCL_SET_ERROR_MSG("Attempted to copy options into itself"); RCL_SET_ERROR_MSG("Attempted to copy options into itself");
return RCL_RET_INVALID_ARGUMENT; 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->domain_id = options->domain_id;
options_out->allocator = options->allocator; options_out->allocator = options->allocator;
options_out->use_global_arguments = options->use_global_arguments; options_out->use_global_arguments = options->use_global_arguments;

21
rcl/test/rcl/test_node.cpp
View file

@ -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_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(&default_options));
EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&not_ini_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: /* Tests special case node_options
// rcl_node_options_t default_options = rcl_node_get_default_options(); */
// rcl_node_options_t not_ini_options; TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options_fail) {
// EXPECT_EQ(RCL_RET_OK, rcl_node_options_copy(&default_options, &not_ini_options)); rcl_node_options_t prev_ini_options = rcl_node_get_default_options();
// EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&not_ini_options)); <-- code fails, returns -11 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));
} }