diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index f4806c8f1..a64fe3541 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -883,21 +883,30 @@ rcl_remove_ros_arguments( int * nonros_argc, const char ** nonros_argv[]) { - RCL_CHECK_ARGUMENT_FOR_NULL(argv, RCL_RET_INVALID_ARGUMENT); - RCL_CHECK_ARGUMENT_FOR_NULL(nonros_argc, RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(args, RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(nonros_argc, RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(nonros_argv, RCL_RET_INVALID_ARGUMENT); + if (NULL != *nonros_argv) { + RCL_SET_ERROR_MSG("Output nonros_argv pointer is not null. May leak memory."); + return RCL_RET_INVALID_ARGUMENT; + } *nonros_argc = rcl_arguments_get_count_unparsed(args); - *nonros_argv = NULL; - - if (*nonros_argc <= 0) { + if (*nonros_argc < 0) { + RCL_SET_ERROR_MSG("Failed to get unparsed non ROS specific arguments count."); return RCL_RET_INVALID_ARGUMENT; + } else if (*nonros_argc > 0) { + RCL_CHECK_ARGUMENT_FOR_NULL(argv, RCL_RET_INVALID_ARGUMENT); + } + + *nonros_argv = NULL; + if (0 == *nonros_argc) { + return RCL_RET_OK; } int * unparsed_indices = NULL; - rcl_ret_t ret; - ret = rcl_arguments_get_unparsed(args, allocator, &unparsed_indices); + rcl_ret_t ret = rcl_arguments_get_unparsed(args, allocator, &unparsed_indices); if (RCL_RET_OK != ret) { return ret; diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index a600c78ba..2dc9a9758 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -570,6 +570,70 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_fini_twice) { rcl_reset_error(); } +TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_bad_remove_ros_args) { + const char * argv[] = {"process_name"}; + int argc = sizeof(argv) / sizeof(const char *); + + rcl_allocator_t default_allocator = rcl_get_default_allocator(); + rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); + rcl_ret_t ret = rcl_parse_arguments(argc, argv, default_allocator, &parsed_args); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); + + const char ** nonros_argv = NULL; + int nonros_argc = 0; + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remove_ros_arguments( + NULL, &parsed_args, default_allocator, &nonros_argc, &nonros_argv)); + rcl_reset_error(); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remove_ros_arguments( + argv, NULL, default_allocator, &nonros_argc, &nonros_argv)); + rcl_reset_error(); + + rcl_allocator_t zero_initialized_allocator = + (rcl_allocator_t)rcutils_get_zero_initialized_allocator(); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remove_ros_arguments( + argv, &parsed_args, zero_initialized_allocator, &nonros_argc, &nonros_argv)); + rcl_reset_error(); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remove_ros_arguments( + argv, &parsed_args, default_allocator, NULL, &nonros_argv)); + rcl_reset_error(); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remove_ros_arguments( + argv, &parsed_args, default_allocator, &nonros_argc, NULL)); + rcl_reset_error(); + + rcl_arguments_t zero_initialized_parsed_args = rcl_get_zero_initialized_arguments(); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remove_ros_arguments( + argv, &zero_initialized_parsed_args, default_allocator, &nonros_argc, &nonros_argv)); + rcl_reset_error(); + + const char * stack_allocated_nonros_argv[] = {"--foo", "--bar"}; + const char ** initialized_nonros_argv = stack_allocated_nonros_argv; + int initialized_nonros_argc = sizeof(stack_allocated_nonros_argv) / sizeof(const char *); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_remove_ros_arguments( + argv, &parsed_args, default_allocator, &initialized_nonros_argc, &initialized_nonros_argv)); + rcl_reset_error(); + + rcl_arguments_t no_parsed_args = rcl_get_zero_initialized_arguments(); + ret = rcl_parse_arguments(0, NULL, default_allocator, &no_parsed_args); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&no_parsed_args)); + }); + + ret = rcl_remove_ros_arguments( + NULL, &no_parsed_args, default_allocator, &nonros_argc, &nonros_argv); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_TRUE(NULL == nonros_argv); + EXPECT_EQ(0, nonros_argc); +} + TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args) { const char * argv[] = { "process_name", "-d", "--ros-args", "-r", "__ns:=/foo/bar", "-r", "__ns:=/fiz/buz", "--", @@ -579,12 +643,17 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args rcl_allocator_t alloc = rcl_get_default_allocator(); rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); - rcl_ret_t ret; - ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); + rcl_ret_t ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); int nonros_argc = 0; const char ** nonros_argv = NULL; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + alloc.deallocate(nonros_argv, alloc.state); + }); ret = rcl_remove_ros_arguments( argv, @@ -593,7 +662,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args &nonros_argc, &nonros_argv); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_EQ(nonros_argc, 6); EXPECT_STREQ(nonros_argv[0], "process_name"); EXPECT_STREQ(nonros_argv[1], "-d"); @@ -601,11 +670,6 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args EXPECT_STREQ(nonros_argv[3], "--baz"); EXPECT_STREQ(nonros_argv[4], "--"); EXPECT_STREQ(nonros_argv[5], "arg"); - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); - - if (NULL != nonros_argv) { - alloc.deallocate(nonros_argv, alloc.state); - } } // \deprecated to be removed in F-Turtle @@ -619,12 +683,17 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_deprecat rcl_allocator_t alloc = rcl_get_default_allocator(); rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); - rcl_ret_t ret; - ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); + rcl_ret_t ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); int nonros_argc = 0; const char ** nonros_argv = NULL; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + alloc.deallocate(nonros_argv, alloc.state); + }); ret = rcl_remove_ros_arguments( argv, @@ -633,29 +702,64 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_deprecat &nonros_argc, &nonros_argv); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_EQ(nonros_argc, 5); EXPECT_STREQ(nonros_argv[0], "process_name"); EXPECT_STREQ(nonros_argv[1], "-d"); EXPECT_STREQ(nonros_argv[2], "--foo=bar"); EXPECT_STREQ(nonros_argv[3], "--bar=baz"); EXPECT_STREQ(nonros_argv[4], "arg"); - EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); +} + +TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args_if_ros_only) { + const char * argv[] = {"--ros-args", "--disable-rosout-logs"}; + int argc = sizeof(argv) / sizeof(const char *); - if (NULL != nonros_argv) { + rcl_allocator_t alloc = rcl_get_default_allocator(); + rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); + rcl_ret_t ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); + + int nonros_argc = 0; + const char ** nonros_argv = NULL; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ alloc.deallocate(nonros_argv, alloc.state); - } + }); + + ret = rcl_remove_ros_arguments( + argv, + &parsed_args, + alloc, + &nonros_argc, + &nonros_argv); + + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + EXPECT_EQ(0, nonros_argc); + EXPECT_TRUE(NULL == nonros_argv); } -TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args_zero) { - const char * argv[] = {""}; - rcl_ret_t ret; + +TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args_if_no_args) { + const char ** argv = NULL; + const int argc = 0; rcl_allocator_t alloc = rcl_get_default_allocator(); rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); + rcl_ret_t ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + }); int nonros_argc = 0; const char ** nonros_argv = NULL; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ + alloc.deallocate(nonros_argv, alloc.state); + }); ret = rcl_remove_ros_arguments( argv, @@ -664,22 +768,20 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args &nonros_argc, &nonros_argv); - EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - if (NULL != nonros_argv) { - alloc.deallocate(nonros_argv, alloc.state); - } + EXPECT_EQ(0, nonros_argc); + EXPECT_TRUE(NULL == nonros_argv); } TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_zero) { const char * argv[] = {"process_name", "--ros-args", "-r", "__ns:=/namespace", "random:=arg"}; int argc = sizeof(argv) / sizeof(const char *); - rcl_ret_t ret; rcl_allocator_t alloc = rcl_get_default_allocator(); rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); - ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); + rcl_ret_t ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); @@ -696,12 +798,11 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_argument_ "--params-file", parameters_filepath.c_str() }; int argc = sizeof(argv) / sizeof(const char *); - rcl_ret_t ret; rcl_allocator_t alloc = rcl_get_default_allocator(); rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); - ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); + rcl_ret_t ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT({ EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args));