Simplify the various 'is_valid' calls

rcl/src/rcl/client.c

@@ -421,11 +421,8 @@ bool
 rcl_client_is_valid(const rcl_client_t * client)
 {
   RCL_CHECK_FOR_NULL_WITH_MSG(client, "client pointer is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    client->impl, "client's rmw implementation is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    client->impl->rmw_handle, "client's rmw handle is invalid", return false);
-  return true;
+
+  return client->impl != NULL && client->impl->rmw_handle != NULL;
 }
 
 const rmw_qos_profile_t *

rcl/src/rcl/event.c

@@ -220,13 +220,13 @@ bool
 rcl_event_is_valid(const rcl_event_t * event)
 {
   RCL_CHECK_FOR_NULL_WITH_MSG(event, "event pointer is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(event->impl, "event's implementation is invalid", return false);
-  if (event->impl->rmw_handle.event_type == RMW_EVENT_INVALID) {
-    RCUTILS_SET_ERROR_MSG("event's implementation not init");
+
+  if (event->impl == NULL || event->impl->rmw_handle.event_type == RMW_EVENT_INVALID) {
     return false;
   }
-  RCUTILS_CHECK_ALLOCATOR_WITH_MSG(
-    &event->impl->allocator, "not valid allocator", return false);
+
+  RCL_CHECK_ALLOCATOR(&event->impl->allocator, return false);
+
   return true;
 }
 

rcl/src/rcl/node.c

@@ -429,24 +429,16 @@ bool
 rcl_node_is_valid_except_context(const rcl_node_t * node)
 {
   RCL_CHECK_FOR_NULL_WITH_MSG(node, "rcl node pointer is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(node->impl, "rcl node implementation is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    node->impl->rmw_node_handle, "rcl node's rmw handle is invalid", return false);
-  return true;
+
+  return node->impl != NULL && node->impl->rmw_node_handle != NULL;
 }
 
 bool
 rcl_node_is_valid(const rcl_node_t * node)
 {
-  bool result = rcl_node_is_valid_except_context(node);
-  if (!result) {
-    return result;
-  }
-  if (!rcl_context_is_valid(node->context)) {
-    RCL_SET_ERROR_MSG("rcl node's context is invalid");
-    return false;
-  }
-  return true;
+  RCL_CHECK_FOR_NULL_WITH_MSG(node, "rcl node pointer is invalid", return false);
+
+  return rcl_node_is_valid_except_context(node) && rcl_context_is_valid(node->context);
 }
 
 const char *

rcl/src/rcl/publisher.c

@@ -417,27 +417,19 @@ rcl_publisher_get_context(const rcl_publisher_t * publisher)
 bool
 rcl_publisher_is_valid(const rcl_publisher_t * publisher)
 {
-  if (!rcl_publisher_is_valid_except_context(publisher)) {
-    return false;  // error already set
-  }
-  if (!rcl_context_is_valid(publisher->impl->context)) {
-    RCL_SET_ERROR_MSG("publisher's context is invalid");
-    return false;
-  }
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    publisher->impl->rmw_handle, "publisher's rmw handle is invalid", return false);
-  return true;
+  RCL_CHECK_FOR_NULL_WITH_MSG(publisher, "publisher pointer is invalid", return false);
+
+  return rcl_publisher_is_valid_except_context(publisher) &&
+         rcl_context_is_valid(publisher->impl->context) &&
+         publisher->impl->rmw_handle != NULL;
 }
 
 bool
 rcl_publisher_is_valid_except_context(const rcl_publisher_t * publisher)
 {
   RCL_CHECK_FOR_NULL_WITH_MSG(publisher, "publisher pointer is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    publisher->impl, "publisher implementation is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    publisher->impl->rmw_handle, "publisher's rmw handle is invalid", return false);
-  return true;
+
+  return publisher->impl != NULL && publisher->impl->rmw_handle != NULL;
 }
 
 rcl_ret_t

rcl/src/rcl/service.c

@@ -423,11 +423,8 @@ bool
 rcl_service_is_valid(const rcl_service_t * service)
 {
   RCL_CHECK_FOR_NULL_WITH_MSG(service, "service pointer is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    service->impl, "service's implementation is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    service->impl->rmw_handle, "service's rmw handle is invalid", return false);
-  return true;
+
+  return service->impl != NULL && service->impl->rmw_handle != NULL;
 }
 
 const rmw_qos_profile_t *

rcl/src/rcl/service_event_publisher.c

@@ -40,14 +40,9 @@ rcl_service_event_publisher_is_valid(const rcl_service_event_publisher_t * servi
 {
   RCL_CHECK_FOR_NULL_WITH_MSG(
     service_event_publisher, "service_event_publisher is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    service_event_publisher->service_type_support,
-    "service_event_publisher's service type support is invalid", return false);
-  if (!rcl_clock_valid(service_event_publisher->clock)) {
-    RCL_SET_ERROR_MSG("service_event_publisher's clock is invalid");
-    return false;
-  }
-  return true;
+
+  return service_event_publisher->service_type_support != NULL &&
+         rcl_clock_valid(service_event_publisher->clock);
 }
 
 static rcl_ret_t introspection_create_publisher(

rcl/src/rcl/subscription.c

@@ -761,11 +761,8 @@ bool
 rcl_subscription_is_valid(const rcl_subscription_t * subscription)
 {
   RCL_CHECK_FOR_NULL_WITH_MSG(subscription, "subscription pointer is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    subscription->impl, "subscription's implementation is invalid", return false);
-  RCL_CHECK_FOR_NULL_WITH_MSG(
-    subscription->impl->rmw_handle, "subscription's rmw handle is invalid", return false);
-  return true;
+
+  return subscription->impl != NULL && subscription->impl->rmw_handle != NULL;
 }
 
 rmw_ret_t

rcl/src/rcl/wait.c

@@ -84,7 +84,9 @@ rcl_get_zero_initialized_wait_set()
 bool
 rcl_wait_set_is_valid(const rcl_wait_set_t * wait_set)
 {
-  return wait_set && wait_set->impl;
+  RCL_CHECK_FOR_NULL_WITH_MSG(wait_set, "wait set pointer is invalid", return false);
+
+  return wait_set->impl != NULL;
 }
 
 static void

rcl/test/rcl/test_events.cpp

@@ -289,15 +289,15 @@ wait_for_msgs_and_events(
   EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
 
   if (nullptr != subscription) {
-    ret = rcl_wait_set_add_subscription(&wait_set, subscription, NULL);
+    ret = rcl_wait_set_add_subscription(&wait_set, subscription, nullptr);
     EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
   }
   if (nullptr != subscription_event) {
-    ret = rcl_wait_set_add_event(&wait_set, subscription_event, NULL);
+    ret = rcl_wait_set_add_event(&wait_set, subscription_event, nullptr);
     EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
   }
   if (nullptr != publisher_event) {
-    ret = rcl_wait_set_add_event(&wait_set, publisher_event, NULL);
+    ret = rcl_wait_set_add_event(&wait_set, publisher_event, nullptr);
     EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
   }
 
@@ -712,8 +712,6 @@ TEST_F(TestEventFixture, test_event_is_valid)
   setup_publisher_subscriber(default_qos_profile, default_qos_profile);
   rcl_event_t publisher_event_test = rcl_get_zero_initialized_event();
   EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test));
-  EXPECT_TRUE(rcl_error_is_set());
-  rcl_reset_error();
 
   rcl_ret_t ret = rcl_publisher_event_init(
     &publisher_event_test, &publisher, RCL_PUBLISHER_OFFERED_DEADLINE_MISSED);
@@ -723,16 +721,12 @@ TEST_F(TestEventFixture, test_event_is_valid)
   rmw_event_type_t saved_event_type = publisher_event_test.impl->rmw_handle.event_type;
   publisher_event_test.impl->rmw_handle.event_type = RMW_EVENT_INVALID;
   EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test));
-  EXPECT_TRUE(rcl_error_is_set());
-  rcl_reset_error();
   publisher_event_test.impl->rmw_handle.event_type = saved_event_type;
 
   rcl_allocator_t saved_alloc = publisher_event_test.impl->allocator;
   rcl_allocator_t bad_alloc = rcutils_get_zero_initialized_allocator();
   publisher_event_test.impl->allocator = bad_alloc;
   EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test));
-  EXPECT_TRUE(rcl_error_is_set());
-  rcl_reset_error();
   publisher_event_test.impl->allocator = saved_alloc;
 
   ret = rcl_event_fini(&publisher_event_test);
@@ -744,18 +738,17 @@ TEST_F(TestEventFixture, test_event_is_valid)
  * Test passing not init to take_event/get_handle
  */
 TEST_F(TestEventFixture, test_event_is_invalid) {
-  // nullptr
   rmw_offered_deadline_missed_status_t deadline_status;
-  EXPECT_EQ(RCL_RET_EVENT_INVALID, rcl_take_event(NULL, &deadline_status));
+  EXPECT_EQ(RCL_RET_EVENT_INVALID, rcl_take_event(nullptr, &deadline_status));
   rcl_reset_error();
-  EXPECT_EQ(NULL, rcl_event_get_rmw_handle(NULL));
+  EXPECT_EQ(nullptr, rcl_event_get_rmw_handle(nullptr));
   rcl_reset_error();
 
   // Zero Init, invalid
   rcl_event_t publisher_event_test = rcl_get_zero_initialized_event();
   EXPECT_EQ(RCL_RET_EVENT_INVALID, rcl_take_event(&publisher_event_test, &deadline_status));
   rcl_reset_error();
-  EXPECT_EQ(NULL, rcl_event_get_rmw_handle(&publisher_event_test));
+  EXPECT_EQ(nullptr, rcl_event_get_rmw_handle(&publisher_event_test));
   rcl_reset_error();
 }
 

rcl/test/rcl/test_node.cpp

@@ -283,8 +283,6 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors)
   rcl_reset_error();
   ret = rcl_node_get_domain_id(&zero_node, &actual_domain_id);
   EXPECT_EQ(RCL_RET_NODE_INVALID, ret);
-  ASSERT_TRUE(rcl_error_is_set());
-  rcl_reset_error();
   ret = rcl_node_get_domain_id(&invalid_node, &actual_domain_id);
   EXPECT_EQ(RCL_RET_NODE_INVALID, ret);
   rcl_reset_error();

rcl_action/test/rcl_action/test_action_client.cpp

@@ -381,7 +381,6 @@ TEST_F(TestActionClientBaseFixture, test_action_client_init_fini_maybe_fail)
         rcl_reset_error();
       }
     } else {
-      EXPECT_TRUE(rcl_error_is_set());
       rcl_reset_error();
     }
     EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node));