From d37c7c987482387443ad3400968749941fa1798a Mon Sep 17 00:00:00 2001
From: Jorge Perez <jjperez@ekumenlabs.com>
Date: Mon, 27 Jul 2020 13:09:56 -0300
Subject: [PATCH] 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>
---
 rcl/src/rcl/event.c          |  6 ++++--
 rcl/test/rcl/test_events.cpp | 23 +++++++++++++++--------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c
index acba993a2..6fc432345 100644
--- 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
index 568d13022..60d0ef015 100644
--- 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
  */