Cleanup error reporting in the type hash code. (#1084)

In particular, using the RCL_CHECK_* macros in both
rcl_node_type_cache_register_type() and
rcl_convert_type_description_runtime_to_msg() meant that
we would overwrite the error message of the latter in
the former.  This would end up with an rcutils warning
about losing error messages

Avoid that by just doing an open-coded check for NULL
if rcl_node_type_cache_register_type(), passing along the
error we got from the lower layers.

While we are in here, also very slightly revamp the
client_init code to not have two separate calls to
type_support->get_type_hash_func().

Signed-off-by: Chris Lalancette <[email protected]>

rcl/src/rcl/client.c

@@ -178,17 +178,23 @@ rcl_client_init(
   client->impl->options = *options;
   atomic_init(&client->impl->sequence_number, 0);
 
+  const rosidl_type_hash_t * hash = type_support->get_type_hash_func(type_support);
+  if (hash == NULL) {
+    RCL_SET_ERROR_MSG("Failed to get the type hash");
+    ret = RCL_RET_INVALID_ARGUMENT;
+    goto destroy_client;
+  }
+
   if (RCL_RET_OK != rcl_node_type_cache_register_type(
-      node, type_support->get_type_hash_func(type_support),
-      type_support->get_type_description_func(type_support),
+      node, hash, type_support->get_type_description_func(type_support),
       type_support->get_type_description_sources_func(type_support)))
   {
     rcutils_reset_error();
     RCL_SET_ERROR_MSG("Failed to register type for client");
     ret = RCL_RET_ERROR;
     goto destroy_client;
   }
-  client->impl->type_hash = *type_support->get_type_hash_func(type_support);
+  client->impl->type_hash = *hash;
 
   RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Client initialized");
   TRACETOOLS_TRACEPOINT(

rcl/src/rcl/node_type_cache.c

@@ -163,19 +163,20 @@ rcl_ret_t rcl_node_type_cache_register_type(
     // Convert type description struct to type description message struct.
     type_info_with_registrations.type_info.type_description =
       rcl_convert_type_description_runtime_to_msg(type_description);
-    RCL_CHECK_FOR_NULL_WITH_MSG(
-      type_info_with_registrations.type_info.type_description,
-      "converting type description struct failed", return RCL_RET_ERROR);
+    if (type_info_with_registrations.type_info.type_description == NULL) {
+      // rcl_convert_type_description_runtime_to_msg already does rcutils_set_error
+      return RCL_RET_ERROR;
+    }
 
     // Convert type sources struct to type sources message struct.
     type_info_with_registrations.type_info.type_sources =
       rcl_convert_type_source_sequence_runtime_to_msg(type_description_sources);
-    RCL_CHECK_FOR_NULL_WITH_MSG(
-      type_info_with_registrations.type_info.type_sources,
-      "converting type sources struct failed",
+    if (type_info_with_registrations.type_info.type_sources == NULL) {
+      // rcl_convert_type_source_sequence_runtime_to_msg already does rcutils_set_error
       type_description_interfaces__msg__TypeDescription__destroy(
         type_info_with_registrations.type_info.type_description);
-      return RCL_RET_ERROR);
+      return RCL_RET_ERROR;
+    }
   } else {
     return RCL_RET_ERROR;
   }

rcl/test/rcl/test_client.cpp

@@ -338,6 +338,7 @@ TEST_F(TestClientFixture, test_client_init_fini_maybe_fail)
       EXPECT_TRUE(rcl_client_is_valid(&client));
       ret = rcl_client_fini(&client, this->node_ptr);
       if (RCL_RET_OK != ret) {
+        rcl_reset_error();
         // If fault injection caused fini to fail, we should try it again.
         EXPECT_EQ(RCL_RET_OK, rcl_client_fini(&client, this->node_ptr));
       }