ARROW-3605: [Plasma] Remove dependence of plasma/events.h on ae.h.

robertnishihara · pcmoritz · commit b4f7ed6d6ed5 · 2018-10-24T23:20:55.000-07:00
Author: Robert Nishihara <robertnishihara@gmail.com> Closes apache#2826 from robertnishihara/aeinclude and squashes the following commits: aa7177b <Robert Nishihara> Address comments. d6ba51f <Robert Nishihara> Remove dependence of plasma/events.h on ae.h.
diff --git a/cpp/src/plasma/events.cc b/cpp/src/plasma/events.cc
@@ -21,8 +21,18 @@
 
 #include <errno.h>
 
+extern "C" {
+#include "ae/ae.h"
+}
+
 namespace plasma {
 
+// Verify that the constants defined in events.h are defined correctly.
+static_assert(kEventLoopTimerDone == AE_NOMORE, "constant defined incorrectly");
+static_assert(kEventLoopOk == AE_OK, "constant defined incorrectly");
+static_assert(kEventLoopRead == AE_READABLE, "constant defined incorrectly");
+static_assert(kEventLoopWrite == AE_WRITABLE, "constant defined incorrectly");
+
 void EventLoop::FileEventCallback(aeEventLoop* loop, int fd, void* context, int events) {
   FileCallback* callback = reinterpret_cast<FileCallback*>(context);
   (*callback)(events);
diff --git a/cpp/src/plasma/events.h b/cpp/src/plasma/events.h
@@ -22,20 +22,24 @@
 #include <memory>
 #include <unordered_map>
 
-extern "C" {
-#include "ae/ae.h"
-}
+struct aeEventLoop;
 
 namespace plasma {
 
+// The constants below are defined using hardcoded values taken from ae.h so
+// that ae.h does not need to be included in this file.
+
 /// Constant specifying that the timer is done and it will be removed.
-constexpr int kEventLoopTimerDone = AE_NOMORE;
+constexpr int kEventLoopTimerDone = -1;  // AE_NOMORE
+
+/// A successful status.
+constexpr int kEventLoopOk = 0;  // AE_OK
 
 /// Read event on the file descriptor.
-constexpr int kEventLoopRead = AE_READABLE;
+constexpr int kEventLoopRead = 1;  // AE_READABLE
 
 /// Write event on the file descriptor.
-constexpr int kEventLoopWrite = AE_WRITABLE;
+constexpr int kEventLoopWrite = 2;  // AE_WRITABLE
 
 typedef long long TimerID;  // NOLINT
 
@@ -57,29 +61,29 @@ class EventLoop {
 
   /// Add a new file event handler to the event loop.
   ///
-  /// @param fd The file descriptor we are listening to.
-  /// @param events The flags for events we are listening to (read or write).
-  /// @param callback The callback that will be called when the event happens.
-  /// @return Returns true if the event handler was added successfully.
+  /// \param fd The file descriptor we are listening to.
+  /// \param events The flags for events we are listening to (read or write).
+  /// \param callback The callback that will be called when the event happens.
+  /// \return Returns true if the event handler was added successfully.
   bool AddFileEvent(int fd, int events, const FileCallback& callback);
 
   /// Remove a file event handler from the event loop.
   ///
-  /// @param fd The file descriptor of the event handler.
+  /// \param fd The file descriptor of the event handler.
   void RemoveFileEvent(int fd);
 
   /// Register a handler that will be called after a time slice of
-  ///  "timeout" milliseconds.
+  /// "timeout" milliseconds.
   ///
-  ///  @param timeout The timeout in milliseconds.
-  ///  @param callback The callback for the timeout.
-  ///  @return The ID of the newly created timer.
+  /// \param timeout The timeout in milliseconds.
+  /// \param callback The callback for the timeout.
+  /// \return The ID of the newly created timer.
   int64_t AddTimer(int64_t timeout, const TimerCallback& callback);
 
   /// Remove a timer handler from the event loop.
   ///
-  /// @param timer_id The ID of the timer that is to be removed.
-  /// @return The ae.c error code. TODO(pcm): needs to be standardized
+  /// \param timer_id The ID of the timer that is to be removed.
+  /// \return The ae.c error code. TODO(pcm): needs to be standardized
   int RemoveTimer(int64_t timer_id);
 
   /// \brief Run the event loop.
diff --git a/cpp/src/plasma/store.cc b/cpp/src/plasma/store.cc
@@ -280,7 +280,7 @@ void PlasmaStore::RemoveGetRequest(GetRequest* get_request) {
   }
   // Remove the get request.
   if (get_request->timer != -1) {
-    ARROW_CHECK(loop_->RemoveTimer(get_request->timer) == AE_OK);
+    ARROW_CHECK(loop_->RemoveTimer(get_request->timer) == kEventLoopOk);
   }
   delete get_request;
 }

Original file line number	Diff line number	Diff line change
`@@ -280,7 +280,7 @@ void PlasmaStore::RemoveGetRequest(GetRequest* get_request) {`
`280`	`280`	`}`
`281`	`281`	`// Remove the get request.`
`282`	`282`	`if (get_request->timer != -1) {`
`283`		`- ARROW_CHECK(loop_->RemoveTimer(get_request->timer) == AE_OK);`
	`283`	`+ ARROW_CHECK(loop_->RemoveTimer(get_request->timer) == kEventLoopOk);`
`284`	`284`	`}`
`285`	`285`	`delete get_request;`
`286`	`286`	`}`