quic: remove opaque pointer from Timer callback

Use the more C++-y `std::function` approach which renders opaque pointers unnecessary. PR-URL: #126 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Sep 23, 2019 · 54e9596 · 54e9596
1 parent 930fb13
commit 54e9596
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 31 deletions.
diff --git a/src/node_quic_session-inl.h b/src/node_quic_session-inl.h
@@ -652,16 +652,6 @@ inline int Empty(const ngtcp2_vec* vec, size_t cnt) {
   return i == cnt;
 }
 
-inline void QuicSession::OnIdleTimeoutCB(void* data) {
-  QuicSession* session = static_cast<QuicSession*>(data);
-  session->OnIdleTimeout();
-}
-
-inline void QuicSession::OnRetransmitTimeoutCB(void* data) {
-  QuicSession* session = static_cast<QuicSession*>(data);
-  session->MaybeTimeout();
-}
-
 }  // namespace quic
 }  // namespace node
 

diff --git a/src/node_quic_session.cc b/src/node_quic_session.cc
@@ -64,8 +64,8 @@ QuicSession::QuicSession(
     alpn_(alpn),
     options_(options),
     initial_connection_close_(initial_connection_close),
-    idle_(new Timer(socket->env(), OnIdleTimeoutCB, this)),
-    retransmit_(new Timer(socket->env(), OnRetransmitTimeoutCB, this)),
+    idle_(new Timer(socket->env(), [this]() { OnIdleTimeout(); })),
+    retransmit_(new Timer(socket->env(), [this]() { MaybeTimeout(); })),
     state_(env()->isolate(), IDX_QUIC_SESSION_STATE_COUNT),
     allocator_(this),
     crypto_rx_ack_(

diff --git a/src/node_quic_session.h b/src/node_quic_session.h
@@ -736,9 +736,6 @@ class QuicSession : public AsyncWrap,
       const ngtcp2_pkt_stateless_reset* sr,
       void* user_data);
 
-  static inline void OnIdleTimeoutCB(void* data);
-  static inline void OnRetransmitTimeoutCB(void* data);
-
   void UpdateIdleTimer();
   void UpdateRetransmitTimer(uint64_t timeout);
   void StopRetransmitTimer();

diff --git a/src/node_quic_util.cc b/src/node_quic_util.cc
@@ -19,7 +19,7 @@ void Timer::Free(Timer* timer) {
 
 void Timer::OnTimeout(uv_timer_t* timer) {
   Timer* t = ContainerOf(&Timer::timer_, timer);
-  t->OnTimeout();
+  t->fn_();
 }
 
 void Timer::CleanupHook(void* data) {

diff --git a/src/node_quic_util.h b/src/node_quic_util.h
@@ -393,14 +393,10 @@ void IncrementStat(
 // reset the timer; Stop to halt the timer.
 class Timer {
  public:
-  inline explicit Timer(
-      Environment* env,
-      std::function<void(void* data)> fn,
-      void* data = nullptr) :
-      stopped_(false),
+  explicit Timer(Environment* env, std::function<void()> fn)
+    : stopped_(false),
       env_(env),
-      fn_(fn),
-      data_(data) {
+      fn_(fn) {
     uv_timer_init(env_->event_loop(), &timer_);
     timer_.data = this;
     env->AddCleanupHook(CleanupHook, this);
@@ -412,7 +408,7 @@ class Timer {
 
   // Stops the timer with the side effect of the timer no longer being usable.
   // It will be cleaned up and the Timer object will be destroyed.
-  inline void Stop() {
+  void Stop() {
     if (stopped_)
       return;
     stopped_ = true;
@@ -425,7 +421,7 @@ class Timer {
 
   // If the timer is not currently active, interval must be either 0 or greater.
   // If the timer is already active, interval is ignored.
-  inline void Update(uint64_t interval) {
+  void Update(uint64_t interval) {
     if (stopped_)
       return;
     uv_timer_start(&timer_, OnTimeout, interval, interval);
@@ -435,18 +431,13 @@ class Timer {
   static void Free(Timer* timer);
 
  private:
-  inline void OnTimeout() {
-    fn_(data_);
-  }
-
   static void OnTimeout(uv_timer_t* timer);
   static void CleanupHook(void* data);
 
   bool stopped_;
   Environment* env_;
-  std::function<void(void* data)> fn_;
+  std::function<void()> fn_;
   uv_timer_t timer_;
-  void* data_;
 };
 
 using TimerPointer = DeleteFnPtr<Timer, Timer::Free>;