Make sure to check the return value of rcl APIs. (#838)

This is just a further check to ensure the test is correct, and also gets rid of a slew of dead store warnings from clang static analysis. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
ros2 · Oct 30, 2020 · f9cea34 · f9cea34
1 parent f3a7318
commit f9cea34
Show file tree

Hide file tree

Showing 14 changed files with 50 additions and 3 deletions.
diff --git a/rcl/test/rcl/client_fixture.cpp b/rcl/test/rcl/client_fixture.cpp
@@ -54,6 +54,11 @@ int main(int argc, char ** argv)
       }
     });
     ret = rcl_init_options_fini(&init_options);
+    if (ret != RCL_RET_OK) {
+      RCUTILS_LOG_ERROR_NAMED(
+        ROS_PACKAGE_NAME, "Error in options fini: %s", rcl_get_error_string().str);
+      return -1;
+    }
     rcl_node_t node = rcl_get_zero_initialized_node();
     const char * name = "client_fixture_node";
     rcl_node_options_t node_options = rcl_node_get_default_options();

diff --git a/rcl/test/rcl/service_fixture.cpp b/rcl/test/rcl/service_fixture.cpp
@@ -59,6 +59,11 @@ int main(int argc, char ** argv)
       }
     });
     ret = rcl_init_options_fini(&init_options);
+    if (ret != RCL_RET_OK) {
+      RCUTILS_LOG_ERROR_NAMED(
+        ROS_PACKAGE_NAME, "Error in options fini: %s", rcl_get_error_string().str);
+      return -1;
+    }
     rcl_node_t node = rcl_get_zero_initialized_node();
     const char * name = "service_fixture_node";
     rcl_node_options_t node_options = rcl_node_get_default_options();

diff --git a/rcl/test/rcl/test_count_matched.cpp b/rcl/test/rcl/test_count_matched.cpp
@@ -126,6 +126,7 @@ class CLASSNAME (TestCountFixture, RMW_IMPLEMENTATION) : public ::testing::Test
     *this->wait_set_ptr = rcl_get_zero_initialized_wait_set();
     ret = rcl_wait_set_init(
       this->wait_set_ptr, 0, 1, 0, 0, 0, 0, this->context_ptr, rcl_get_default_allocator());
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
   }
 
   void TearDown()

diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp
@@ -888,6 +888,7 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME
     this->remote_context_ptr = new rcl_context_t;
     *this->remote_context_ptr = rcl_get_zero_initialized_context();
     ret = rcl_init(0, nullptr, &init_options, this->remote_context_ptr);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
 
     ret = rcl_node_init(
       remote_node_ptr, remote_node_name, "", this->remote_context_ptr,

diff --git a/rcl/test/rcl/test_rmw_impl_id_check_exe.cpp b/rcl/test/rcl/test_rmw_impl_id_check_exe.cpp
@@ -27,6 +27,9 @@ int main(int, char **)
     return ret;
   }
   ret = rcl_init_options_fini(&init_options);
+  if (ret != RCL_RET_OK) {
+    return ret;
+  }
   ret = rcl_shutdown(&context);
   if (ret != RCL_RET_OK) {
     return ret;

diff --git a/rcl/test/rcl/test_subscription.cpp b/rcl/test/rcl/test_subscription.cpp
@@ -420,7 +420,9 @@ TEST_F(
     test_msgs__msg__Strings__init(&msg);
     ASSERT_TRUE(rosidl_runtime_c__String__assign(&msg.string_value, test_string));
     ret = rcl_publish(&publisher, &msg, nullptr);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_publish(&publisher, &msg, nullptr);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_publish(&publisher, &msg, nullptr);
     test_msgs__msg__Strings__fini(&msg);
     ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
@@ -488,9 +490,13 @@ TEST_F(
     test_msgs__msg__Strings__init(&msg);
     ASSERT_TRUE(rosidl_runtime_c__String__assign(&msg.string_value, test_string));
     ret = rcl_publish(&publisher, &msg, nullptr);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_publish(&publisher, &msg, nullptr);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_publish(&publisher, &msg, nullptr);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_publish(&publisher, &msg, nullptr);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_publish(&publisher, &msg, nullptr);
     test_msgs__msg__Strings__fini(&msg);
     ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;

diff --git a/rcl/test/test_namespace.cpp b/rcl/test/test_namespace.cpp
@@ -106,6 +106,7 @@ TEST_F(TestNamespaceFixture, test_client_server) {
   bool is_available = false;
   for (auto i = 0; i < timeout; ++i) {
     ret = rcl_service_server_is_available(this->node_ptr, &unmatched_client, &is_available);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     if (is_available) {
       // this should not happen
       break;
@@ -128,6 +129,7 @@ TEST_F(TestNamespaceFixture, test_client_server) {
   is_available = false;
   for (auto i = 0; i < timeout; ++i) {
     ret = rcl_service_server_is_available(this->node_ptr, &matched_client, &is_available);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     if (is_available) {
       break;
     }

diff --git a/rcl_action/test/rcl_action/test_action_client.cpp b/rcl_action/test/rcl_action/test_action_client.cpp
@@ -165,7 +165,6 @@ TEST_F(TestActionClientBaseFixture, test_action_client_init_fini) {
   EXPECT_EQ(ret, RCL_RET_BAD_ALLOC) << rcl_get_error_string().str;
   rcl_reset_error();
 
-  ret = RCL_RET_OK;
   int i = 0;
   do {
     time_bomb_state.malloc_count_until_failure = i;

diff --git a/rcl_action/test/rcl_action/test_action_communication.cpp b/rcl_action/test/rcl_action/test_action_communication.cpp
@@ -55,6 +55,7 @@ class CLASSNAME (TestActionCommunication, RMW_IMPLEMENTATION) : public ::testing
     ret = rcl_node_init(&this->node, "test_action_communication_node", "", &context, &node_options);
     ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_clock_init(RCL_STEADY_TIME, &this->clock, &allocator);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     const rosidl_action_type_support_t * ts = ROSIDL_GET_ACTION_TYPE_SUPPORT(
       test_msgs, Fibonacci);
     const char * action_name = "test_action_commmunication_name";
@@ -227,6 +228,7 @@ TEST_F(CLASSNAME(TestActionCommunication, RMW_IMPLEMENTATION), test_valid_goal_c
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -346,6 +348,7 @@ TEST_F(CLASSNAME(TestActionCommunication, RMW_IMPLEMENTATION), test_valid_cancel
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -463,6 +466,7 @@ TEST_F(CLASSNAME(TestActionCommunication, RMW_IMPLEMENTATION), test_valid_result
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -535,6 +539,7 @@ TEST_F(CLASSNAME(TestActionCommunication, RMW_IMPLEMENTATION), test_valid_status
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);

diff --git a/rcl_action/test/rcl_action/test_action_interaction.cpp b/rcl_action/test/rcl_action/test_action_interaction.cpp
@@ -67,6 +67,7 @@ class CLASSNAME (TestActionClientServerInteraction, RMW_IMPLEMENTATION) : public
     ret = rcl_node_init(&this->node, "test_action_communication_node", "", &context, &node_options);
     ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     ret = rcl_clock_init(RCL_STEADY_TIME, &this->clock, &allocator);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     const rosidl_action_type_support_t * ts = ROSIDL_GET_ACTION_TYPE_SUPPORT(
       test_msgs, Fibonacci);
     const char * action_name = "test_action_commmunication_name";
@@ -216,6 +217,7 @@ TEST_F(CLASSNAME(TestActionClientServerInteraction, RMW_IMPLEMENTATION), test_in
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&this->wait_set, &this->action_server, NULL);
   ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
@@ -261,6 +263,7 @@ TEST_F(CLASSNAME(TestActionClientServerInteraction, RMW_IMPLEMENTATION), test_in
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -319,6 +322,7 @@ TEST_F(CLASSNAME(TestActionClientServerInteraction, RMW_IMPLEMENTATION), test_in
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -363,6 +367,7 @@ TEST_F(CLASSNAME(TestActionClientServerInteraction, RMW_IMPLEMENTATION), test_in
       this->outgoing_feedback.feedback.sequence.size));
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&this->wait_set, &this->action_server, NULL);
   ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
@@ -413,6 +418,7 @@ TEST_F(CLASSNAME(TestActionClientServerInteraction, RMW_IMPLEMENTATION), test_in
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -484,6 +490,7 @@ TEST_F(
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&this->wait_set, &this->action_server, NULL);
   ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
@@ -529,6 +536,7 @@ TEST_F(
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -587,6 +595,7 @@ TEST_F(
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -631,6 +640,7 @@ TEST_F(
       this->outgoing_feedback.feedback.sequence.size));
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&this->wait_set, &this->action_server, NULL);
   ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
@@ -687,6 +697,7 @@ TEST_F(
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&this->wait_set, &this->action_server, NULL);
   ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
@@ -737,6 +748,7 @@ TEST_F(
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);
@@ -786,6 +798,7 @@ TEST_F(
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_wait_set_clear(&this->wait_set);
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_client(
     &this->wait_set, &this->action_client, NULL, NULL);

diff --git a/rcl_action/test/rcl_action/test_graph.cpp b/rcl_action/test/rcl_action/test_graph.cpp
@@ -296,6 +296,7 @@ class TestActionGraphMultiNodeFixture : public CLASSNAME(TestActionGraphFixture,
 
     this->remote_context = rcl_get_zero_initialized_context();
     ret = rcl_init(0, nullptr, &init_options, &this->remote_context);
+    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
 
     ret = rcl_node_init(
       &this->remote_node, this->remote_node_name, "", &this->remote_context, &node_options);

diff --git a/rcl_action/test/rcl_action/test_wait.cpp b/rcl_action/test/rcl_action/test_wait.cpp
@@ -294,6 +294,7 @@ TEST_F(TestActionServerWait, test_wait_set_add_action_server) {
   EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str;
   ret = rcl_wait_set_init(
     &wait_set, 0, 0, 0, 0, 0, 0, &this->context, rcl_get_default_allocator());
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&wait_set, &this->action_server, &service_index);
   EXPECT_EQ(ret, RCL_RET_WAIT_SET_FULL) << rcl_get_error_string().str;
@@ -305,6 +306,7 @@ TEST_F(TestActionServerWait, test_wait_set_add_action_server) {
   EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str;
   ret = rcl_wait_set_init(
     &wait_set, 0, 0, 0, 0, 1, 0, &this->context, rcl_get_default_allocator());
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&wait_set, &this->action_server, &service_index);
   EXPECT_EQ(ret, RCL_RET_WAIT_SET_FULL) << rcl_get_error_string().str;
@@ -316,6 +318,7 @@ TEST_F(TestActionServerWait, test_wait_set_add_action_server) {
   EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str;
   ret = rcl_wait_set_init(
     &wait_set, 0, 0, 0, 0, 2, 0, &this->context, rcl_get_default_allocator());
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&wait_set, &this->action_server, &service_index);
   EXPECT_EQ(ret, RCL_RET_WAIT_SET_FULL) << rcl_get_error_string().str;
@@ -327,6 +330,7 @@ TEST_F(TestActionServerWait, test_wait_set_add_action_server) {
   EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str;
   ret = rcl_wait_set_init(
     &wait_set, 0, 0, 0, 0, 3, 0, &this->context, rcl_get_default_allocator());
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&wait_set, &this->action_server, &service_index);
   EXPECT_EQ(ret, RCL_RET_WAIT_SET_FULL) << rcl_get_error_string().str;
@@ -338,6 +342,7 @@ TEST_F(TestActionServerWait, test_wait_set_add_action_server) {
   EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str;
   ret = rcl_wait_set_init(
     &wait_set, 0, 0, 1, 0, 3, 0, &this->context, rcl_get_default_allocator());
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   ret = rcl_action_wait_set_add_action_server(&wait_set, &this->action_server, &service_index);
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
@@ -347,6 +352,7 @@ TEST_F(TestActionServerWait, test_wait_set_add_action_server) {
   EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str;
   ret = rcl_wait_set_init(
     &wait_set, 0, 0, 1, 0, 3, 0, &this->context, rcl_get_default_allocator());
+  ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
   ret = rcl_action_wait_set_add_action_server(&wait_set, &this->action_server, nullptr);
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
   EXPECT_FALSE(rcl_error_is_set()) << rcl_get_error_string().str;

diff --git a/rcl_lifecycle/test/test_rcl_lifecycle.cpp b/rcl_lifecycle/test/test_rcl_lifecycle.cpp
@@ -69,6 +69,7 @@ TEST(TestRclLifecycle, lifecycle_state) {
   rcutils_reset_error();
 
   ret = rcl_lifecycle_state_init(&state, expected_id, &expected_label[0], &allocator);
+  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
   EXPECT_EQ(expected_id, state.id);
   EXPECT_STREQ(&expected_label[0], state.label);
 

diff --git a/rcl_lifecycle/test/test_transition_map.cpp b/rcl_lifecycle/test/test_transition_map.cpp
@@ -73,6 +73,7 @@ TEST_F(TestTransitionMap, initialized) {
 
   rcl_lifecycle_state_t state1 = {"my_state_1", 1, NULL, 0};
   ret = rcl_lifecycle_register_state(&transition_map, state1, &allocator);
+  ASSERT_EQ(RCL_RET_OK, ret);
 
   rcl_lifecycle_state_t unregistered = {"my_state_2", 2, NULL, 0};
 
@@ -96,15 +97,13 @@ TEST_F(TestTransitionMap, initialized) {
 
   rcl_lifecycle_transition_t transition01 = {"from0to1", 0,
     start_state, goal_state};
-  original_size = transition_map.transitions_size;
   ret = rcl_lifecycle_register_transition(
     &transition_map, transition01, &allocator);
   EXPECT_EQ(RCL_RET_OK, ret);
   EXPECT_EQ(1u, transition_map.transitions_size);
 
   rcl_lifecycle_transition_t transition10 = {"from1to0", 1,
     goal_state, start_state};
-  original_size = transition_map.transitions_size;
   ret = rcl_lifecycle_register_transition(
     &transition_map, transition10, &allocator);
   EXPECT_EQ(RCL_RET_OK, ret);