Force rcl_arguments_t to be zero initialized. (#225)

* 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.

rcl/include/rcl/arguments.h

@@ -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()
  *
@@ -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

rcl/src/rcl/arguments.c

@@ -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;
 

rcl/test/rcl/arg_macros.hpp

@@ -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( \

rcl/test/rcl/test_arguments.cpp

@@ -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);
@@ -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));
@@ -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();
@@ -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();
@@ -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();
@@ -161,7 +161,7 @@ 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();
@@ -169,13 +169,36 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_two_namespace)
   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();
@@ -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));
@@ -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();