Skip to content

Commit

Permalink
Force rcl_arguments_t to be zero initialized. (#225)
Browse files Browse the repository at this point in the history 
* Force rcl_arguments_t to be zero initialized.
* Adds a check for rcl_arguments_t->impl to be NULL before use.
* Updates tests to account for zero initialization.
  • Loading branch information
@mjcarroll
mjcarroll authored Apr 11, 2018
1 parent d41c923 commit c51f892
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 9 deletions.
3 changes: 3 additions & 0 deletions rcl/include/rcl/arguments.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ rcl_get_zero_initialized_arguments(void);
/**
* If an argument does not appear to be a valid ROS argument then it is skipped
* and parsing continues with the next argument in `argv`.
*
* \sa rcl_get_zero_initialized_arguments()
* \sa rcl_arguments_get_count_unparsed()
* \sa rcl_arguments_get_unparsed()
*
Expand All @@ -67,6 +69,7 @@ rcl_get_zero_initialized_arguments(void);
* \param[in] argv The values of the arguments.
* \param[in] allocator A valid allocator.
* \param[out] args_output A structure that will contain the result of parsing.
* Must be zero initialized before use.
* \return `RCL_RET_OK` if the arguments were parsed successfully, or
* \return `RCL_RET_INVALID_ARGUMENT` if any function arguments are invalid, or
* \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or
Expand Down
6 changes: 6 additions & 0 deletions rcl/src/rcl/arguments.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,18 @@ rcl_parse_arguments(
{
RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
if (argc < 0) {
RCL_SET_ERROR_MSG("Argument count cannot be negative", allocator);
return RCL_RET_INVALID_ARGUMENT;
} else if (argc > 0) {
RCL_CHECK_ARGUMENT_FOR_NULL(argv, RCL_RET_INVALID_ARGUMENT, allocator);
}
RCL_CHECK_ARGUMENT_FOR_NULL(args_output, RCL_RET_INVALID_ARGUMENT, allocator);

if (args_output->impl != NULL) {
RCL_SET_ERROR_MSG("Parse output is not zero-initialized", allocator);
return RCL_RET_INVALID_ARGUMENT;
}

rcl_ret_t ret;
rcl_ret_t fail_ret;

Expand Down
1 change: 1 addition & 0 deletions rcl/test/rcl/arg_macros.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ destroy_args(int argc, char ** args)

#define SCOPE_ARGS(local_arguments, ...) \
{ \
local_arguments = rcl_get_zero_initialized_arguments(); \
const char * local_argv[] = {__VA_ARGS__}; \
unsigned int local_argc = (sizeof(local_argv) / sizeof(const char *)); \
rcl_ret_t ret = rcl_parse_arguments( \
Expand Down
41 changes: 32 additions & 9 deletions rcl/test/rcl/test_arguments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ bool
is_valid_arg(const char * arg)
{
const char * argv[] = {arg};
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret = rcl_parse_arguments(1, argv, rcl_get_default_allocator(), &parsed_args);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
bool is_valid = 0 == rcl_arguments_get_count_unparsed(&parsed_args);
Expand Down Expand Up @@ -113,7 +113,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_inval
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_no_args) {
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret = rcl_parse_arguments(0, NULL, rcl_get_default_allocator(), &parsed_args);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_EQ(0, rcl_arguments_get_count_unparsed(&parsed_args));
Expand All @@ -122,7 +122,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_no_args) {

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_null_args) {
int argc = 1;
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret = rcl_parse_arguments(argc, NULL, rcl_get_default_allocator(), &parsed_args);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret) << rcl_get_error_string_safe();
rcl_reset_error();
Expand All @@ -139,7 +139,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_null_args_outpu
TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_one_remap) {
const char * argv[] = {"process_name", "/foo/bar:=/fiz/buz"};
int argc = sizeof(argv) / sizeof(const char *);
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret;
ret = rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
Expand All @@ -150,7 +150,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_one_remap) {
TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_mix_valid_invalid_rules) {
const char * argv[] = {"process_name", "/foo/bar:=", "bar:=/fiz/buz", "}bar:=fiz"};
int argc = sizeof(argv) / sizeof(const char *);
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret;
ret = rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
Expand All @@ -161,21 +161,44 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_mix_valid_inval
TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_two_namespace) {
const char * argv[] = {"process_name", "__ns:=/foo/bar", "__ns:=/fiz/buz"};
int argc = sizeof(argv) / sizeof(const char *);
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret;
ret = rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
EXPECT_UNPARSED(parsed_args, 0);
EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args));
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_uninitialized_parsed_args) {
const char * argv[] = {"process_name"};
int argc = sizeof(argv) / sizeof(const char *);
rcl_arguments_t parsed_args;
int not_null = 1;
parsed_args.impl = reinterpret_cast<rcl_arguments_impl_t *>(&not_null);
ASSERT_EQ(RCL_RET_INVALID_ARGUMENT,
rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args));
rcl_reset_error();
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_double_parse) {
const char * argv[] = {"process_name", "__ns:=/foo/bar", "__ns:=/fiz/buz"};
int argc = sizeof(argv) / sizeof(const char *);
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
ASSERT_EQ(RCL_RET_OK,
rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args));
ASSERT_EQ(RCL_RET_INVALID_ARGUMENT,
rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args));
rcl_reset_error();
}


TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_fini_null) {
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_arguments_fini(NULL));
rcl_reset_error();
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_fini_impl_null) {
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
parsed_args.impl = NULL;
EXPECT_EQ(RCL_RET_ERROR, rcl_arguments_fini(&parsed_args));
rcl_reset_error();
Expand All @@ -184,7 +207,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_fini_impl_null)
TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_fini_twice) {
const char * argv[] = {"process_name"};
int argc = sizeof(argv) / sizeof(const char *);
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
ASSERT_EQ(RCL_RET_OK, rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &parsed_args));
EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args));
EXPECT_EQ(RCL_RET_ERROR, rcl_arguments_fini(&parsed_args));
Expand All @@ -197,7 +220,7 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_remove_ros_args
int argc = sizeof(argv) / sizeof(const char *);

rcl_allocator_t alloc = rcl_get_default_allocator();
rcl_arguments_t parsed_args;
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret;
ret = rcl_parse_arguments(argc, argv, alloc, &parsed_args);
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string_safe();
Expand Down

0 comments on commit c51f892

Please sign in to comment.