Add extra check for invalid event implementation (#726)

* Add tests proving the required functionality * Add check to the functions * Change name to test Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
ros2 · Oct 28, 2020 · 257ddfc · 257ddfc
1 parent 0ec13a2
commit 257ddfc
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 10 deletions.
diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c
@@ -137,7 +137,9 @@ rcl_take_event(
   void * event_info)
 {
   bool taken = false;
-  RCL_CHECK_ARGUMENT_FOR_NULL(event, RCL_RET_EVENT_INVALID);
+  if (!rcl_event_is_valid(event)) {
+    return RCL_RET_EVENT_INVALID;
+  }
   RCL_CHECK_ARGUMENT_FOR_NULL(event_info, RCL_RET_INVALID_ARGUMENT);
   rmw_ret_t ret = rmw_take_event(&event->impl->rmw_handle, event_info, &taken);
   if (RMW_RET_OK != ret) {
@@ -179,7 +181,7 @@ rcl_event_fini(rcl_event_t * event)
 rmw_event_t *
 rcl_event_get_rmw_handle(const rcl_event_t * event)
 {
-  if (NULL == event) {
+  if (!rcl_event_is_valid(event)) {
     return NULL;  // error already set
   } else {
     return &event->impl->rmw_handle;

diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp
@@ -725,14 +725,6 @@ TEST_F(TestEventFixture, test_bad_event_ini)
   tear_down_publisher_subscriber();
 }
 
-/*
- * Passing bad argument to get_rmw_handle
- */
-TEST_F(TestEventFixture, test_bad_get_handle)
-{
-  EXPECT_EQ(NULL, rcl_event_get_rmw_handle(NULL));
-}
-
 /*
  * Test cases for the event_is_valid function
  */
@@ -773,6 +765,21 @@ TEST_F(TestEventFixture, test_event_is_valid)
   tear_down_publisher_subscriber();
 }
 
+/*
+ * 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(NULL, rcl_event_get_rmw_handle(NULL));
+
+  // 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));
+  EXPECT_EQ(NULL, rcl_event_get_rmw_handle(&publisher_event_test));
+}
+
 /*
  * Basic test subscriber event message lost
  */