Skip to content

Commit

Permalink
Handle zero non-ROS specific args properly in rcl_remove_ros_arguments (
Browse files Browse the repository at this point in the history 
#518)

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
  • Loading branch information
@hidmic
hidmic authored Oct 15, 2019
1 parent 5fb711c commit 1a840d1
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 32 deletions.
23 changes: 16 additions & 7 deletions rcl/src/rcl/arguments.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
151 changes: 126 additions & 25 deletions rcl/test/rcl/test_arguments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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", "--",
Expand All @@ -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,
Expand All @@ -593,19 +662,14 @@ 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");
EXPECT_STREQ(nonros_argv[2], "--foo=bar");
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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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));
Expand All @@ -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));
Expand Down

0 comments on commit 1a840d1

Please sign in to comment.