From c07cee46d30b0eef625e2edf4ae967e49cd61b94 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Sat, 7 Nov 2020 03:27:14 +0800 Subject: [PATCH] refactor for removing unnecessary source code (#857) * refactor for removing unnecessary source code Co-authored-by: Chris Lalancette Signed-off-by: Chen Lihui --- rcl/src/rcl/init_options.c | 52 ++++++++++++++++++-------------------- rcl/test/rcl/test_init.cpp | 17 ------------- 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/rcl/src/rcl/init_options.c b/rcl/src/rcl/init_options.c index 9f88b2166..5fa93c70d 100644 --- a/rcl/src/rcl/init_options.c +++ b/rcl/src/rcl/init_options.c @@ -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) { @@ -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); @@ -74,6 +88,7 @@ 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"); @@ -81,32 +96,13 @@ rcl_init_options_copy(const rcl_init_options_t * src, rcl_init_options_t * dst) } // 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(); diff --git a/rcl/test/rcl/test_init.cpp b/rcl/test/rcl/test_init.cpp index dd3c119c0..72913c258 100644 --- a/rcl/test/rcl/test_init.cpp +++ b/rcl/test/rcl/test_init.cpp @@ -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();