Skip to content

Commit

Permalink
Fix bug error handling get_param_files (#743)
Browse files Browse the repository at this point in the history
* Add bomb allocator
* Fix memory checked for error
* Add tests for bad alloc
* Fix break statement
* Add static keyword to methods added

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
  • Loading branch information
Blast545 authored Aug 13, 2020
1 parent f90c01c commit ed449a5
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 4 deletions.
5 changes: 1 addition & 4 deletions rcl/src/rcl/arguments.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,7 @@ rcl_arguments_get_param_files(
if (NULL == (*parameter_files)[i]) {
// deallocate allocated memory
for (int r = i; r >= 0; --r) {
if (NULL == (*parameter_files[r])) {
break;
}
allocator.deallocate((*parameter_files[r]), allocator.state);
allocator.deallocate((*parameter_files)[r], allocator.state);
}
allocator.deallocate((*parameter_files), allocator.state);
(*parameter_files) = NULL;
Expand Down
74 changes: 74 additions & 0 deletions rcl/test/rcl/allocator_testing_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,80 @@ set_failing_allocator_is_failing(rcutils_allocator_t & failing_allocator, bool s
((__failing_allocator_state *)failing_allocator.state)->is_failing = state;
}

typedef struct time_bomb_allocator_state
{
int count_until_failure;
} time_bomb_allocator_state;

static void * time_bomb_malloc(size_t size, void * state)
{
time_bomb_allocator_state * time_bomb_state = (time_bomb_allocator_state *)state;
if (time_bomb_state->count_until_failure >= 0 &&
time_bomb_state->count_until_failure-- == 0)
{
return nullptr;
}
return rcutils_get_default_allocator().allocate(size, rcutils_get_default_allocator().state);
}

static void *
time_bomb_realloc(void * pointer, size_t size, void * state)
{
time_bomb_allocator_state * time_bomb_state = (time_bomb_allocator_state *)state;
if (time_bomb_state->count_until_failure >= 0 &&
time_bomb_state->count_until_failure-- == 0)
{
return nullptr;
}
return rcutils_get_default_allocator().reallocate(
pointer, size, rcutils_get_default_allocator().state);
}

static void
time_bomb_free(void * pointer, void * state)
{
time_bomb_allocator_state * time_bomb_state = (time_bomb_allocator_state *)state;
if (time_bomb_state->count_until_failure >= 0 &&
time_bomb_state->count_until_failure-- == 0)
{
return;
}
rcutils_get_default_allocator().deallocate(pointer, rcutils_get_default_allocator().state);
}

static void *
time_bomb_calloc(size_t number_of_elements, size_t size_of_element, void * state)
{
time_bomb_allocator_state * time_bomb_state = (time_bomb_allocator_state *)state;
if (time_bomb_state->count_until_failure >= 0 &&
time_bomb_state->count_until_failure-- == 0)
{
return nullptr;
}
return rcutils_get_default_allocator().zero_allocate(
number_of_elements, size_of_element, rcutils_get_default_allocator().state);
}

static inline rcutils_allocator_t
get_time_bombed_allocator(void)
{
static time_bomb_allocator_state state;
state.count_until_failure = 1;
auto time_bombed_allocator = rcutils_get_default_allocator();
time_bombed_allocator.allocate = time_bomb_malloc;
time_bombed_allocator.deallocate = time_bomb_free;
time_bombed_allocator.reallocate = time_bomb_realloc;
time_bombed_allocator.zero_allocate = time_bomb_calloc;
time_bombed_allocator.state = &state;
return time_bombed_allocator;
}

static inline void
set_time_bombed_allocator_count(rcutils_allocator_t & time_bombed_allocator, int count)
{
((time_bomb_allocator_state *)time_bombed_allocator.state)->count_until_failure = count;
}

#ifdef __cplusplus
}
#endif
Expand Down
36 changes: 36 additions & 0 deletions rcl/test/rcl/test_arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1029,3 +1029,39 @@ TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_param_overrides
ASSERT_TRUE(NULL != param_value->string_value);
EXPECT_STREQ("foo", param_value->string_value);
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_bad_alloc_get_param_files) {
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()
};
const int argc = sizeof(argv) / sizeof(const char *);

rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret = rcl_parse_arguments(argc, argv, rcl_get_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));
});

int parameter_filecount = rcl_arguments_get_param_files_count(&parsed_args);
EXPECT_EQ(2, parameter_filecount);

// Configure allocator to fail at different points of the code
rcl_allocator_t bomb_alloc = get_time_bombed_allocator();
set_time_bombed_allocator_count(bomb_alloc, 0);
char ** parameter_files = NULL;
ret = rcl_arguments_get_param_files(&parsed_args, bomb_alloc, &parameter_files);
EXPECT_EQ(RCL_RET_BAD_ALLOC, ret) << rcl_get_error_string().str;

set_time_bombed_allocator_count(bomb_alloc, 1);
ret = rcl_arguments_get_param_files(&parsed_args, bomb_alloc, &parameter_files);
EXPECT_EQ(RCL_RET_BAD_ALLOC, ret) << rcl_get_error_string().str;

set_time_bombed_allocator_count(bomb_alloc, 2);
ret = rcl_arguments_get_param_files(&parsed_args, bomb_alloc, &parameter_files);
EXPECT_EQ(RCL_RET_BAD_ALLOC, ret) << rcl_get_error_string().str;
}

0 comments on commit ed449a5

Please sign in to comment.