Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor for removing unnecessary source code #857

Merged
merged 3 commits into from
Nov 6, 2020
Merged

refactor for removing unnecessary source code #857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f7f8b2
refactor for removing unnecessary source code
Nov 5, 2020
4fcec36
Add a helper function that make rcl_init_options_copy not to call rmw…
Nov 5, 2020
7ccbbe1
use inline for the helper function and rename it to look more clear
Nov 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 24 additions & 28 deletions rcl/src/rcl/init_options.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,22 @@ rcl_get_zero_initialized_init_options(void)
}; // NOLINT(readability/braces): false positive
}

/// Initialize given init_options with the default values and zero-initialize implementation.
static inline
rcl_ret_t
_rcl_init_options_zero_init(rcl_init_options_t * init_options, rcl_allocator_t allocator)
{
init_options->impl = allocator.allocate(sizeof(rcl_init_options_impl_t), allocator.state);
RCL_CHECK_FOR_NULL_WITH_MSG(
init_options->impl,
"failed to allocate memory for init options impl",
return RCL_RET_BAD_ALLOC);
init_options->impl->allocator = allocator;
init_options->impl->rmw_init_options = rmw_get_zero_initialized_init_options();

return RCL_RET_OK;
}

rcl_ret_t
rcl_init_options_init(rcl_init_options_t * init_options, rcl_allocator_t allocator)
{
Expand All @@ -48,13 +64,11 @@ rcl_init_options_init(rcl_init_options_t * init_options, rcl_allocator_t allocat
return RCL_RET_ALREADY_INIT;
}
RCL_CHECK_ALLOCATOR(&allocator, return RCL_RET_INVALID_ARGUMENT);
init_options->impl = allocator.allocate(sizeof(rcl_init_options_impl_t), allocator.state);
RCL_CHECK_FOR_NULL_WITH_MSG(
init_options->impl,
"failed to allocate memory for init options impl",
return RCL_RET_BAD_ALLOC);
init_options->impl->allocator = allocator;
init_options->impl->rmw_init_options = rmw_get_zero_initialized_init_options();

rcl_ret_t ret = _rcl_init_options_zero_init(init_options, allocator);
if (RCL_RET_OK != ret) {
return ret;
}
rmw_ret_t rmw_ret = rmw_init_options_init(&(init_options->impl->rmw_init_options), allocator);
if (RMW_RET_OK != rmw_ret) {
allocator.deallocate(init_options->impl, allocator.state);
Expand All @@ -74,39 +88,21 @@ rcl_init_options_copy(const rcl_init_options_t * src, rcl_init_options_t * dst)

RCL_CHECK_ARGUMENT_FOR_NULL(src, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(src->impl, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ALLOCATOR(&src->impl->allocator, return RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(dst, RCL_RET_INVALID_ARGUMENT);
if (NULL != dst->impl) {
RCL_SET_ERROR_MSG("given dst (rcl_init_options_t) is already initialized");
return RCL_RET_ALREADY_INIT;
}

// initialize dst (since we know it's in a zero initialized state)
rcl_ret_t ret = rcl_init_options_init(dst, src->impl->allocator);
rcl_ret_t ret = _rcl_init_options_zero_init(dst, src->impl->allocator);
if (RCL_RET_OK != ret) {
return ret; // error already set
}

// copy src information into dst
dst->impl->allocator = src->impl->allocator;
// first zero-initialize rmw init options
rmw_ret_t rmw_ret = rmw_init_options_fini(&(dst->impl->rmw_init_options));
if (RMW_RET_OK != rmw_ret) {
rmw_error_string_t error_string = rmw_get_error_string();
rmw_reset_error();
ret = rcl_init_options_fini(dst);
if (RCL_RET_OK != ret) {
RCUTILS_LOG_ERROR_NAMED(
"rcl",
"failed to finalize dst rcl_init_options while handling failure to "
"finalize rmw_init_options, original ret '%d' and error: %s", rmw_ret, error_string.str);
return ret; // error already set
}
RCL_SET_ERROR_MSG(error_string.str);
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
}
// then copy
dst->impl->rmw_init_options = rmw_get_zero_initialized_init_options();
rmw_ret =
rmw_ret_t rmw_ret =
rmw_init_options_copy(&(src->impl->rmw_init_options), &(dst->impl->rmw_init_options));
if (RMW_RET_OK != rmw_ret) {
rmw_error_string_t error_string = rmw_get_error_string();
Expand Down
17 changes: 0 additions & 17 deletions rcl/test/rcl/test_init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,23 +548,6 @@ TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_mocked_rcl_init_optio
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options));
}

// Mock rcl_init_options_copy to fail
TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_rcl_init_copy_mocked_fail_fini) {
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
rcl_ret_t ret = rcl_init_options_init(&init_options, rcl_get_default_allocator());
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str;
});
rcl_init_options_t init_options_dst = rcl_get_zero_initialized_init_options();
auto mock = mocking_utils::inject_on_return("lib:rcl", rmw_init_options_fini, RMW_RET_ERROR);
EXPECT_EQ(RCL_RET_ERROR, rcl_init_options_copy(&init_options, &init_options_dst));
rcl_reset_error();
auto mock_ok = mocking_utils::inject_on_return("lib:rcl", rmw_init_options_fini, RMW_RET_OK);
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options_dst));
}

// Mock rcl_init_options_copy to fail
TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_rcl_init_options_copy_fail_rmw_copy) {
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
Expand Down