diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index 67e08c8471..c33886feb8 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -499,44 +499,52 @@ rcl_arguments_copy( // Zero so it's safe to call rcl_arguments_fini() if an error occurrs while copying. args_out->impl->num_remap_rules = 0; + args_out->impl->remap_rules = NULL; + args_out->impl->unparsed_args = NULL; args_out->impl->num_unparsed_args = 0; + args_out->impl->parameter_files = NULL; args_out->impl->num_param_files_args = 0; - // Copy unparsed args - args_out->impl->unparsed_args = allocator.allocate( - sizeof(int) * args->impl->num_unparsed_args, allocator.state); - if (NULL == args_out->impl->unparsed_args) { - if (RCL_RET_OK != rcl_arguments_fini(args_out)) { - RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error"); + if (args->impl->num_unparsed_args) { + // Copy unparsed args + args_out->impl->unparsed_args = allocator.allocate( + sizeof(int) * args->impl->num_unparsed_args, allocator.state); + if (NULL == args_out->impl->unparsed_args) { + if (RCL_RET_OK != rcl_arguments_fini(args_out)) { + RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error"); + } + return RCL_RET_BAD_ALLOC; } - return RCL_RET_BAD_ALLOC; - } - for (int i = 0; i < args->impl->num_unparsed_args; ++i) { - args_out->impl->unparsed_args[i] = args->impl->unparsed_args[i]; - } - args_out->impl->num_unparsed_args = args->impl->num_unparsed_args; - - // Copy remap rules - args_out->impl->remap_rules = allocator.allocate( - sizeof(rcl_remap_t) * args->impl->num_remap_rules, allocator.state); - if (NULL == args_out->impl->remap_rules) { - if (RCL_RET_OK != rcl_arguments_fini(args_out)) { - RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error"); + for (int i = 0; i < args->impl->num_unparsed_args; ++i) { + args_out->impl->unparsed_args[i] = args->impl->unparsed_args[i]; } - return RCL_RET_BAD_ALLOC; + args_out->impl->num_unparsed_args = args->impl->num_unparsed_args; } - args_out->impl->num_remap_rules = args->impl->num_remap_rules; - for (int i = 0; i < args->impl->num_remap_rules; ++i) { - args_out->impl->remap_rules[i] = rcl_remap_get_zero_initialized(); - rcl_ret_t ret = rcl_remap_copy( - &(args->impl->remap_rules[i]), &(args_out->impl->remap_rules[i])); - if (RCL_RET_OK != ret) { + + if (args->impl->num_remap_rules) { + // Copy remap rules + args_out->impl->remap_rules = allocator.allocate( + sizeof(rcl_remap_t) * args->impl->num_remap_rules, allocator.state); + if (NULL == args_out->impl->remap_rules) { if (RCL_RET_OK != rcl_arguments_fini(args_out)) { RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error"); } - return ret; + return RCL_RET_BAD_ALLOC; + } + args_out->impl->num_remap_rules = args->impl->num_remap_rules; + for (int i = 0; i < args->impl->num_remap_rules; ++i) { + args_out->impl->remap_rules[i] = rcl_remap_get_zero_initialized(); + rcl_ret_t ret = rcl_remap_copy( + &(args->impl->remap_rules[i]), &(args_out->impl->remap_rules[i])); + if (RCL_RET_OK != ret) { + if (RCL_RET_OK != rcl_arguments_fini(args_out)) { + RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error"); + } + return ret; + } } } + // Copy parameter files if (args->impl->num_param_files_args) { args_out->impl->parameter_files = allocator.allocate( @@ -558,8 +566,6 @@ rcl_arguments_copy( return RCL_RET_BAD_ALLOC; } } - } else { - args_out->impl->parameter_files = NULL; } return RCL_RET_OK; } diff --git a/rcl/test/rcl/test_arguments.cpp b/rcl/test/rcl/test_arguments.cpp index 42df6e01f0..a4baa645e5 100644 --- a/rcl/test/rcl/test_arguments.cpp +++ b/rcl/test/rcl/test_arguments.cpp @@ -202,6 +202,36 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_copy) { EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&copied_args)); } +// Similar to the default allocator, but returns NULL when size is zero. +// This is useful for emulating systems where `malloc(0)` return NULL. +// TODO(jacobperron): Consider using this allocate function in other tests +static void * +__return_null_on_zero_allocate(size_t size, void * state) +{ + RCUTILS_UNUSED(state); + if (size == 0) { + return NULL; + } + return malloc(size); +} + +TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_copy_no_args) { + rcl_allocator_t allocator = rcl_get_default_allocator(); + allocator.allocate = __return_null_on_zero_allocate; + rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments(); + rcl_ret_t ret = rcl_parse_arguments(0, NULL, allocator, &parsed_args); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(0, rcl_arguments_get_count_unparsed(&parsed_args)); + + rcl_arguments_t copied_args = rcl_get_zero_initialized_arguments(); + ret = rcl_arguments_copy(&parsed_args, &copied_args); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(0, rcl_arguments_get_count_unparsed(&copied_args)); + + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&parsed_args)); + EXPECT_EQ(RCL_RET_OK, rcl_arguments_fini(&copied_args)); +} + 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 *);