Ensure compliant init options API implementations. (#399)

Signed-off-by: Michel Hidalgo <[email protected]>
ros2 · Oct 8, 2020 · ef4b43d · ef4b43d
1 parent a02b048
commit ef4b43d
Showing 1 changed file with 31 additions and 19 deletions.
diff --git a/rmw_fastrtps_shared_cpp/src/rmw_init.cpp b/rmw_fastrtps_shared_cpp/src/rmw_init.cpp
@@ -29,6 +29,7 @@ rmw_ret_t
 rmw_init_options_init(
   const char * identifier, rmw_init_options_t * init_options, rcutils_allocator_t allocator)
 {
+  assert(identifier != NULL);
   RMW_CHECK_ARGUMENT_FOR_NULL(init_options, RMW_RET_INVALID_ARGUMENT);
   RCUTILS_CHECK_ALLOCATOR(&allocator, return RMW_RET_INVALID_ARGUMENT);
   if (NULL != init_options->implementation_identifier) {
@@ -50,8 +51,13 @@ rmw_ret_t
 rmw_init_options_copy(
   const char * identifier, const rmw_init_options_t * src, rmw_init_options_t * dst)
 {
+  assert(identifier != NULL);
   RMW_CHECK_ARGUMENT_FOR_NULL(src, RMW_RET_INVALID_ARGUMENT);
   RMW_CHECK_ARGUMENT_FOR_NULL(dst, RMW_RET_INVALID_ARGUMENT);
+  if (NULL == src->implementation_identifier) {
+    RMW_SET_ERROR_MSG("expected initialized dst");
+    return RMW_RET_INVALID_ARGUMENT;
+  }
   RMW_CHECK_TYPE_IDENTIFIERS_MATCH(
     src,
     src->implementation_identifier,
@@ -62,39 +68,45 @@ rmw_init_options_copy(
     return RMW_RET_INVALID_ARGUMENT;
   }
   const rcutils_allocator_t * allocator = &src->allocator;
-  rmw_ret_t ret = RMW_RET_OK;
-
-  allocator->deallocate(dst->enclave, allocator->state);
-  *dst = *src;
-  dst->enclave = NULL;
-  dst->security_options = rmw_get_zero_initialized_security_options();
+  RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT);
 
-  dst->enclave = rcutils_strdup(src->enclave, *allocator);
-  if (src->enclave && !dst->enclave) {
-    ret = RMW_RET_BAD_ALLOC;
-    goto fail;
+  rmw_init_options_t tmp = *src;
+  tmp.enclave = rcutils_strdup(tmp.enclave, *allocator);
+  if (NULL != src->enclave && NULL == tmp.enclave) {
+    return RMW_RET_BAD_ALLOC;
   }
-  return rmw_security_options_copy(&src->security_options, allocator, &dst->security_options);
-fail:
-  allocator->deallocate(dst->enclave, allocator->state);
-  return ret;
+  tmp.security_options = rmw_get_zero_initialized_security_options();
+  rmw_ret_t ret =
+    rmw_security_options_copy(&src->security_options, allocator, &tmp.security_options);
+  if (RMW_RET_OK != ret) {
+    allocator->deallocate(tmp.enclave, allocator->state);
+    return ret;
+  }
+  *dst = tmp;
+  return RMW_RET_OK;
 }
 
 rmw_ret_t
 rmw_init_options_fini(const char * identifier, rmw_init_options_t * init_options)
 {
+  assert(identifier != NULL);
   RMW_CHECK_ARGUMENT_FOR_NULL(init_options, RMW_RET_INVALID_ARGUMENT);
-  rcutils_allocator_t & allocator = init_options->allocator;
-  RCUTILS_CHECK_ALLOCATOR(&allocator, return RMW_RET_INVALID_ARGUMENT);
+  if (NULL == init_options->implementation_identifier) {
+    RMW_SET_ERROR_MSG("expected initialized init_options");
+    return RMW_RET_INVALID_ARGUMENT;
+  }
   RMW_CHECK_TYPE_IDENTIFIERS_MATCH(
     init_options,
     init_options->implementation_identifier,
     identifier,
     return RMW_RET_INCORRECT_RMW_IMPLEMENTATION);
-  allocator.deallocate(init_options->enclave, allocator.state);
-  rmw_security_options_fini(&init_options->security_options, &allocator);
+  rcutils_allocator_t * allocator = &init_options->allocator;
+  RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT);
+
+  allocator->deallocate(init_options->enclave, allocator->state);
+  rmw_ret_t ret = rmw_security_options_fini(&init_options->security_options, allocator);
   *init_options = rmw_get_zero_initialized_init_options();
-  return RMW_RET_OK;
+  return ret;
 }
 
 }  // namespace rmw_fastrtps_shared_cpp