Skip to content

Commit

Permalink
Fix allocation arguments copy (#748)
Browse files Browse the repository at this point in the history
* Change allocation method for copied parameter files
* Add bad allocator test

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
  • Loading branch information
Blast545 authored and ahcorde committed Oct 28, 2020
1 parent d03ebf6 commit a19e816
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
4 changes: 2 additions & 2 deletions rcl/src/rcl/arguments.c
Original file line number Diff line number Diff line change
Expand Up @@ -862,8 +862,8 @@ rcl_arguments_copy(

// Copy parameter files
if (args->impl->num_param_files_args) {
args_out->impl->parameter_files = allocator.allocate(
sizeof(char *) * args->impl->num_param_files_args, allocator.state);
args_out->impl->parameter_files = allocator.zero_allocate(
args->impl->num_param_files_args, sizeof(char *), allocator.state);
if (NULL == args_out->impl->parameter_files) {
if (RCL_RET_OK != rcl_arguments_fini(args_out)) {
RCL_SET_ERROR_MSG("Error while finalizing arguments due to another error");
Expand Down
34 changes: 34 additions & 0 deletions rcl/test/rcl/test_arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1065,3 +1065,37 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_bad_alloc_get_p
ret = rcl_arguments_get_param_files(&parsed_args, bomb_alloc, &parameter_files);
EXPECT_EQ(RCL_RET_BAD_ALLOC, ret) << rcl_get_error_string().str;
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_bad_allocs_copy) {
const std::string parameters_filepath1 = (test_path / "test_parameters.1.yaml").string();
const std::string parameters_filepath2 = (test_path / "test_parameters.2.yaml").string();
const char * const argv[] = {
"process_name", "--ros-args", "--params-file", parameters_filepath1.c_str(),
"-r", "__ns:=/namespace", "random:=arg", "--params-file", parameters_filepath2.c_str(),
"-r", "/foo/bar:=/fiz/buz", "--remap", "foo:=/baz",
"-e", "/foo", "--", "foo"
};
const int argc = sizeof(argv) / sizeof(const char *);

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));
});

rcl_arguments_t copied_args = rcl_get_zero_initialized_arguments();
rcl_allocator_t bomb_alloc = get_time_bombed_allocator();
rcl_allocator_t saved_alloc = parsed_args.impl->allocator;
parsed_args.impl->allocator = bomb_alloc;
for (int i = 0; i < 8; i++) {
set_time_bombed_allocator_count(bomb_alloc, i);
ret = rcl_arguments_copy(&parsed_args, &copied_args);
EXPECT_EQ(RCL_RET_BAD_ALLOC, ret) << rcl_get_error_string().str;
rcl_reset_error();
}
parsed_args.impl->allocator = saved_alloc;
}

0 comments on commit a19e816

Please sign in to comment.