refactor for removing unnecessary source code (#857)

* refactor for removing unnecessary source code Co-authored-by: Chris Lalancette <clalancette@openrobotics.org> Signed-off-by: Chen Lihui <Lihui.Chen@sony.com>
ros2 · Nov 6, 2020 · c07cee4 · c07cee4
1 parent 3472e3c
commit c07cee4
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 45 deletions.
diff --git 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,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();

diff --git 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();