nghttpx: Code cleanup

Mainly make nested code block to rather flat style.

src/shrpx_http2_session.cc

@@ -272,19 +272,25 @@ void eventcb(bufferevent *bev, short events, void *ptr)
       return;
     }
 
-    int fd = bufferevent_getfd(bev);
+    auto fd = bufferevent_getfd(bev);
     int val = 1;
     if(setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
-                  reinterpret_cast<char *>(&val), sizeof(val)) == -1) {
+                  reinterpret_cast<char*>(&val), sizeof(val)) == -1) {
       SSLOG(WARNING, http2session)
         << "Setting option TCP_NODELAY failed: errno=" << errno;
     }
-  } else if(events & BEV_EVENT_EOF) {
+    return;
+  }
+
+  if(events & BEV_EVENT_EOF) {
     if(LOG_ENABLED(INFO)) {
       SSLOG(INFO, http2session) << "EOF";
     }
     http2session->disconnect();
-  } else if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
+    return;
+  }
+
+  if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
     if(LOG_ENABLED(INFO)) {
       if(events & BEV_EVENT_ERROR) {
         SSLOG(INFO, http2session) << "Network error";
@@ -293,6 +299,7 @@ void eventcb(bufferevent *bev, short events, void *ptr)
       }
     }
     http2session->disconnect();
+    return;
   }
 }
 } // namespace
@@ -350,12 +357,18 @@ void proxy_eventcb(bufferevent *bev, short events, void *ptr)
       SSLOG(ERROR, http2session) << "bufferevent_write() failed";
       http2session->disconnect();
     }
-  } else if(events & BEV_EVENT_EOF) {
+    return;
+  }
+
+  if(events & BEV_EVENT_EOF) {
     if(LOG_ENABLED(INFO)) {
       SSLOG(INFO, http2session) << "Proxy EOF";
     }
     http2session->disconnect();
-  } else if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
+    return;
+  }
+
+  if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
     if(LOG_ENABLED(INFO)) {
       if(events & BEV_EVENT_ERROR) {
         SSLOG(INFO, http2session) << "Network error";
@@ -364,6 +377,7 @@ void proxy_eventcb(bufferevent *bev, short events, void *ptr)
       }
     }
     http2session->disconnect();
+    return;
   }
 }
 } // namespace
@@ -409,7 +423,11 @@ int Http2Session::initiate_connection()
     proxy_htp_->data = this;
 
     state_ = PROXY_CONNECTING;
-  } else if(state_ == DISCONNECTED || state_ == PROXY_CONNECTED) {
+
+    return 0;
+  }
+
+  if(state_ == DISCONNECTED || state_ == PROXY_CONNECTED) {
     if(LOG_ENABLED(INFO)) {
       SSLOG(INFO, this) << "Connecting to downstream server";
     }
@@ -492,11 +510,12 @@ int Http2Session::initiate_connection()
     if(state_ != CONNECTED) {
       state_ = CONNECTING;
     }
-  } else {
+
-    // Unreachable
+    return 0;
-    DIE();
   }
-  return 0;
+
+  // Unreachable
+  DIE();
 }
 
 void Http2Session::unwrap_free_bev()
@@ -517,10 +536,13 @@ int htp_hdrs_completecb(http_parser *htp)
       SSLOG(INFO, http2session) << "Tunneling success";
     }
     http2session->set_state(Http2Session::PROXY_CONNECTED);
-  } else {
+
-    SSLOG(WARNING, http2session) << "Tunneling failed";
+    return 0;
-    http2session->set_state(Http2Session::PROXY_FAILED);
   }
+
+  SSLOG(WARNING, http2session) << "Tunneling failed";
+  http2session->set_state(Http2Session::PROXY_FAILED);
+
   return 0;
 }
 } // namespace

src/shrpx_http2_upstream.cc

@@ -61,37 +61,50 @@ int on_stream_close_callback
                          << " is being closed";
   }
   auto downstream = upstream->find_downstream(stream_id);
-  if(downstream) {
-    if(downstream->get_request_state() == Downstream::CONNECT_FAIL) {
-      upstream->remove_downstream(downstream);
-      delete downstream;
-    } else {
-      downstream->set_request_state(Downstream::STREAM_CLOSED);
-      if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
-        // At this point, downstream response was read
-        if(!downstream->get_upgraded() &&
-           !downstream->get_response_connection_close()) {
-          // Keep-alive
-          auto dconn = downstream->get_downstream_connection();
-          if(dconn) {
-            dconn->detach_downstream(downstream);
-          }
-        }
-        upstream->remove_downstream(downstream);
-        delete downstream;
-      } else {
-        // At this point, downstream read may be paused.
 
-        // If shrpx_downstream::push_request_headers() failed, the
+  if(!downstream) {
-        // error is handled here.
+    return 0;
-        upstream->remove_downstream(downstream);
+  }
-        delete downstream;
+
-        // How to test this case? Request sufficient large download
+  if(downstream->get_request_state() == Downstream::CONNECT_FAIL) {
-        // and make client send RST_STREAM after it gets first DATA
+    upstream->remove_downstream(downstream);
-        // frame chunk.
+
+    delete downstream;
+
+    return 0;
+  }
+
+  downstream->set_request_state(Downstream::STREAM_CLOSED);
+
+  if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
+    // At this point, downstream response was read
+    if(!downstream->get_upgraded() &&
+       !downstream->get_response_connection_close()) {
+      // Keep-alive
+      auto dconn = downstream->get_downstream_connection();
+
+      if(dconn) {
+        dconn->detach_downstream(downstream);
       }
     }
+
+    upstream->remove_downstream(downstream);
+
+    delete downstream;
+
+    return 0;
   }
+
+  // At this point, downstream read may be paused.
+
+  // If shrpx_downstream::push_request_headers() failed, the
+  // error is handled here.
+  upstream->remove_downstream(downstream);
+  delete downstream;
+  // How to test this case? Request sufficient large download
+  // and make client send RST_STREAM after it gets first DATA
+  // frame chunk.
+
   return 0;
 }
 } // namespace
@@ -423,12 +436,16 @@ int on_data_chunk_recv_callback(nghttp2_session *session,
 {
   auto upstream = static_cast<Http2Upstream*>(user_data);
   auto downstream = upstream->find_downstream(stream_id);
-  if(downstream) {
+
-    if(downstream->push_upload_data_chunk(data, len) != 0) {
+  if(!downstream) {
-      upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR);
+    return 0;
-      return 0;
-    }
   }
+
+  if(downstream->push_upload_data_chunk(data, len) != 0) {
+    upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR);
+    return 0;
+  }
+
   return 0;
 }
 } // namespace
@@ -691,6 +708,7 @@ void downstream_readcb(bufferevent *bev, void *ptr)
   auto dconn = static_cast<DownstreamConnection*>(ptr);
   auto downstream = dconn->get_downstream();
   auto upstream = static_cast<Http2Upstream*>(downstream->get_upstream());
+
   if(downstream->get_request_state() == Downstream::STREAM_CLOSED) {
     // If upstream HTTP2 stream was closed, we just close downstream,
     // because there is no consumer now. Downstream connection is also
@@ -707,11 +725,11 @@ void downstream_readcb(bufferevent *bev, void *ptr)
     // on_stream_close_callback.
     upstream->rst_stream(downstream, infer_upstream_rst_stream_error_code
                          (downstream->get_response_rst_stream_error_code()));
-    downstream->set_downstream_connection(0);
+    downstream->set_downstream_connection(nullptr);
     delete dconn;
-    dconn = 0;
+    dconn = nullptr;
   } else {
-    int rv = downstream->on_read();
+    auto rv = downstream->on_read();
     if(rv != 0) {
       if(LOG_ENABLED(INFO)) {
         DCLOG(INFO, dconn) << "HTTP parser failure";
@@ -728,9 +746,9 @@ void downstream_readcb(bufferevent *bev, void *ptr)
       downstream->set_response_state(Downstream::MSG_COMPLETE);
       // Clearly, we have to close downstream connection on http parser
       // failure.
-      downstream->set_downstream_connection(0);
+      downstream->set_downstream_connection(nullptr);
       delete dconn;
-      dconn = 0;
+      dconn = nullptr;
     }
   }
   if(upstream->send() != 0) {
@@ -765,14 +783,18 @@ void downstream_eventcb(bufferevent *bev, short events, void *ptr)
       DCLOG(INFO, dconn) << "Connection established. stream_id="
                          << downstream->get_stream_id();
     }
-    int fd = bufferevent_getfd(bev);
+    auto fd = bufferevent_getfd(bev);
     int val = 1;
     if(setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
-                  reinterpret_cast<char *>(&val), sizeof(val)) == -1) {
+                  reinterpret_cast<char*>(&val), sizeof(val)) == -1) {
       DCLOG(WARNING, dconn) << "Setting option TCP_NODELAY failed: errno="
                             << errno;
     }
-  } else if(events & BEV_EVENT_EOF) {
+
+    return;
+  }
+
+  if(events & BEV_EVENT_EOF) {
     if(LOG_ENABLED(INFO)) {
       DCLOG(INFO, dconn) << "EOF. stream_id=" << downstream->get_stream_id();
     }
@@ -781,41 +803,46 @@ void downstream_eventcb(bufferevent *bev, short events, void *ptr)
       // the first place. We can delete downstream.
       upstream->remove_downstream(downstream);
       delete downstream;
-    } else {
-      // Delete downstream connection. If we don't delete it here, it
-      // will be pooled in on_stream_close_callback.
-      downstream->set_downstream_connection(0);
-      delete dconn;
-      dconn = 0;
-      // downstream wil be deleted in on_stream_close_callback.
-      if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
-        // Server may indicate the end of the request by EOF
-        if(LOG_ENABLED(INFO)) {
-          ULOG(INFO, upstream) << "Downstream body was ended by EOF";
-        }
-        downstream->set_response_state(Downstream::MSG_COMPLETE);
 
-        // For tunneled connection, MSG_COMPLETE signals
+      return;
-        // downstream_data_read_callback to send RST_STREAM after
+    }
-        // pending response body is sent. This is needed to ensure
+
-        // that RST_STREAM is sent after all pending data are sent.
+    // Delete downstream connection. If we don't delete it here, it
-        upstream->on_downstream_body_complete(downstream);
+    // will be pooled in on_stream_close_callback.
-      } else if(downstream->get_response_state() != Downstream::MSG_COMPLETE) {
+    downstream->set_downstream_connection(nullptr);
-        // If stream was not closed, then we set MSG_COMPLETE and let
+    delete dconn;
-        // on_stream_close_callback delete downstream.
+    dconn = nullptr;
-        if(upstream->error_reply(downstream, 502) != 0) {
+    // downstream wil be deleted in on_stream_close_callback.
-          delete upstream->get_client_handler();
+    if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
-          return;
+      // Server may indicate the end of the request by EOF
-        }
+      if(LOG_ENABLED(INFO)) {
-        downstream->set_response_state(Downstream::MSG_COMPLETE);
+        ULOG(INFO, upstream) << "Downstream body was ended by EOF";
       }
-      if(upstream->send() != 0) {
+      downstream->set_response_state(Downstream::MSG_COMPLETE);
+
+      // For tunneled connection, MSG_COMPLETE signals
+      // downstream_data_read_callback to send RST_STREAM after
+      // pending response body is sent. This is needed to ensure
+      // that RST_STREAM is sent after all pending data are sent.
+      upstream->on_downstream_body_complete(downstream);
+    } else if(downstream->get_response_state() != Downstream::MSG_COMPLETE) {
+      // If stream was not closed, then we set MSG_COMPLETE and let
+      // on_stream_close_callback delete downstream.
+      if(upstream->error_reply(downstream, 502) != 0) {
         delete upstream->get_client_handler();
         return;
       }
-      // At this point, downstream may be deleted.
+      downstream->set_response_state(Downstream::MSG_COMPLETE);
     }
-  } else if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
+    if(upstream->send() != 0) {
+      delete upstream->get_client_handler();
+      return;
+    }
+    // At this point, downstream may be deleted.
+    return;
+  }
+
+  if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
     if(LOG_ENABLED(INFO)) {
       if(events & BEV_EVENT_ERROR) {
         DCLOG(INFO, dconn) << "Downstream network error: "
@@ -828,45 +855,50 @@ void downstream_eventcb(bufferevent *bev, short events, void *ptr)
         DCLOG(INFO, dconn) << "Note: this is tunnel connection";
       }
     }
+
     if(downstream->get_request_state() == Downstream::STREAM_CLOSED) {
       upstream->remove_downstream(downstream);
       delete downstream;
-    } else {
+
-      // Delete downstream connection. If we don't delete it here, it
+      return;
-      // will be pooled in on_stream_close_callback.
-      downstream->set_downstream_connection(0);
-      delete dconn;
-      dconn = 0;
-      if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
-        // For SSL tunneling, we issue RST_STREAM. For other types of
-        // stream, we don't have to do anything since response was
-        // complete.
-        if(downstream->get_upgraded()) {
-          upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR);
-        }
-      } else {
-        if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
-          upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR);
-        } else {
-          unsigned int status;
-          if(events & BEV_EVENT_TIMEOUT) {
-            status = 504;
-          } else {
-            status = 502;
-          }
-          if(upstream->error_reply(downstream, status) != 0) {
-            delete upstream->get_client_handler();
-            return;
-          }
-        }
-        downstream->set_response_state(Downstream::MSG_COMPLETE);
-      }
-      if(upstream->send() != 0) {
-        delete upstream->get_client_handler();
-        return;
-      }
-      // At this point, downstream may be deleted.
     }
+
+    // Delete downstream connection. If we don't delete it here, it
+    // will be pooled in on_stream_close_callback.
+    downstream->set_downstream_connection(nullptr);
+    delete dconn;
+    dconn = nullptr;
+
+    if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
+      // For SSL tunneling, we issue RST_STREAM. For other types of
+      // stream, we don't have to do anything since response was
+      // complete.
+      if(downstream->get_upgraded()) {
+        upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR);
+      }
+    } else {
+      if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
+        upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR);
+      } else {
+        unsigned int status;
+        if(events & BEV_EVENT_TIMEOUT) {
+          status = 504;
+        } else {
+          status = 502;
+        }
+        if(upstream->error_reply(downstream, status) != 0) {
+          delete upstream->get_client_handler();
+          return;
+        }
+      }
+      downstream->set_response_state(Downstream::MSG_COMPLETE);
+    }
+    if(upstream->send() != 0) {
+      delete upstream->get_client_handler();
+      return;
+    }
+    // At this point, downstream may be deleted.
+    return;
   }
 }
 } // namespace

src/shrpx_https_upstream.cc

@@ -193,19 +193,23 @@ int htp_hdrs_completecb(http_parser *htp)
   }
 
   rv =  dconn->attach_downstream(downstream);
+
   if(rv != 0) {
     downstream->set_request_state(Downstream::CONNECT_FAIL);
-    downstream->set_downstream_connection(0);
+    downstream->set_downstream_connection(nullptr);
     delete dconn;
     return -1;
-  } else {
-    rv = downstream->push_request_headers();
-    if(rv != 0) {
-      return -1;
-    }
-    downstream->set_request_state(Downstream::HEADER_COMPLETE);
-    return 0;
   }
+
+  rv = downstream->push_request_headers();
+
+  if(rv != 0) {
+    return -1;
+  }
+
+  downstream->set_request_state(Downstream::HEADER_COMPLETE);
+
+  return 0;
 }
 } // namespace
 
@@ -431,9 +435,9 @@ int HttpsUpstream::resume_read(IOCtrlReason reason, Downstream *downstream)
     // are not notified by readcb until new data arrive.
     http_parser_pause(&htp_, 0);
     return on_read();
-  } else {
-    return 0;
   }
+
+  return 0;
 }
 
 namespace {
@@ -443,80 +447,109 @@ void https_downstream_readcb(bufferevent *bev, void *ptr)
   auto downstream = dconn->get_downstream();
   auto upstream = static_cast<HttpsUpstream*>(downstream->get_upstream());
   int rv;
+
   rv = downstream->on_read();
+
   if(downstream->get_response_state() == Downstream::MSG_RESET) {
     delete upstream->get_client_handler();
-  } else if(rv == 0) {
-    auto handler = upstream->get_client_handler();
 
-    if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
+    return;
-      if(downstream->get_response_connection_close()) {
+  }
-        // Connection close
+
-        downstream->set_downstream_connection(0);
+  if(rv != 0) {
-        delete dconn;
-        dconn = 0;
-      } else {
-        // Keep-alive
-        dconn->detach_downstream(downstream);
-      }
-      if(downstream->get_request_state() == Downstream::MSG_COMPLETE) {
-        if(handler->get_should_close_after_write() &&
-           handler->get_outbuf_length() == 0) {
-          // If all upstream response body has already written out to
-          // the peer, we cannot use writecb for ClientHandler. In
-          // this case, we just delete handler here.
-          delete handler;
-          return;
-        } else {
-          upstream->delete_downstream();
-          // Process next HTTP request
-          if(upstream->resume_read(SHRPX_MSG_BLOCK, 0) == -1) {
-            return;
-          }
-        }
-      } else if(downstream->get_upgraded()) {
-        // This path is effectively only taken for HTTP2 downstream
-        // because only HTTP2 downstream sets response_state to
-        // MSG_COMPLETE and this function. For HTTP downstream, EOF
-        // from tunnel connection is handled on
-        // https_downstream_eventcb.
-        //
-        // Tunneled connection always indicates connection close.
-        if(handler->get_outbuf_length() == 0) {
-          // For tunneled connection, if there is no pending data,
-          // delete handler because on_write will not be called.
-          delete handler;
-        } else {
-          if(LOG_ENABLED(INFO)) {
-            DLOG(INFO, downstream) << "Tunneled connection has pending data";
-          }
-        }
-      }
-    } else {
-      if(handler->get_outbuf_length() >= OUTBUF_MAX_THRES) {
-        downstream->pause_read(SHRPX_NO_BUFFER);
-      }
-    }
-  } else {
     if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
       // We already sent HTTP response headers to upstream
       // client. Just close the upstream connection.
       delete upstream->get_client_handler();
-    } else {
+
-      // We did not sent any HTTP response, so sent error
+      return;
-      // response. Cannot reuse downstream connection in this case.
+    }
-      if(upstream->error_reply(502) != 0) {
+
-        delete upstream->get_client_handler();
+    // We did not sent any HTTP response, so sent error
+    // response. Cannot reuse downstream connection in this case.
+    if(upstream->error_reply(502) != 0) {
+      delete upstream->get_client_handler();
+
+      return;
+    }
+
+    if(downstream->get_request_state() == Downstream::MSG_COMPLETE) {
+      upstream->delete_downstream();
+
+      // Process next HTTP request
+      if(upstream->resume_read(SHRPX_MSG_BLOCK, 0) == -1) {
         return;
       }
-      if(downstream->get_request_state() == Downstream::MSG_COMPLETE) {
-        upstream->delete_downstream();
-        // Process next HTTP request
-        if(upstream->resume_read(SHRPX_MSG_BLOCK, 0) == -1) {
-          return;
-        }
-      }
     }
+
+    return;
+  }
+
+  auto handler = upstream->get_client_handler();
+
+  if(downstream->get_response_state() != Downstream::MSG_COMPLETE) {
+    if(handler->get_outbuf_length() >= OUTBUF_MAX_THRES) {
+      downstream->pause_read(SHRPX_NO_BUFFER);
+    }
+
+    return;
+  }
+
+
+  if(downstream->get_response_connection_close()) {
+    // Connection close
+    downstream->set_downstream_connection(nullptr);
+
+    delete dconn;
+
+    dconn = nullptr;
+  } else {
+    // Keep-alive
+    dconn->detach_downstream(downstream);
+  }
+
+  if(downstream->get_request_state() == Downstream::MSG_COMPLETE) {
+    if(handler->get_should_close_after_write() &&
+       handler->get_outbuf_length() == 0) {
+      // If all upstream response body has already written out to
+      // the peer, we cannot use writecb for ClientHandler. In
+      // this case, we just delete handler here.
+      delete handler;
+
+      return;
+    }
+
+    upstream->delete_downstream();
+
+    // Process next HTTP request
+    if(upstream->resume_read(SHRPX_MSG_BLOCK, 0) == -1) {
+      return;
+    }
+
+    return;
+  }
+
+  if(downstream->get_upgraded()) {
+    // This path is effectively only taken for HTTP2 downstream
+    // because only HTTP2 downstream sets response_state to
+    // MSG_COMPLETE and this function. For HTTP downstream, EOF
+    // from tunnel connection is handled on
+    // https_downstream_eventcb.
+    //
+    // Tunneled connection always indicates connection close.
+    if(handler->get_outbuf_length() == 0) {
+      // For tunneled connection, if there is no pending data,
+      // delete handler because on_write will not be called.
+      delete handler;
+
+      return;
+    }
+
+    if(LOG_ENABLED(INFO)) {
+      DLOG(INFO, downstream) << "Tunneled connection has pending data";
+    }
+
+    return;
   }
 }
 } // namespace
@@ -545,7 +578,11 @@ void https_downstream_eventcb(bufferevent *bev, short events, void *ptr)
     if(LOG_ENABLED(INFO)) {
       DCLOG(INFO, dconn) << "Connection established";
     }
-  } else if(events & BEV_EVENT_EOF) {
+
+    return;
+  }
+
+  if(events & BEV_EVENT_EOF) {
     if(LOG_ENABLED(INFO)) {
       DCLOG(INFO, dconn) << "EOF";
     }
@@ -567,9 +604,7 @@ void https_downstream_eventcb(bufferevent *bev, short events, void *ptr)
         delete handler;
         return;
       }
-    } else if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
+    } else if(downstream->get_response_state() != Downstream::MSG_COMPLETE) {
-      // Nothing to do
-    } else {
       // error
       if(LOG_ENABLED(INFO)) {
         DCLOG(INFO, dconn) << "Treated as error";
@@ -585,7 +620,11 @@ void https_downstream_eventcb(bufferevent *bev, short events, void *ptr)
         return;
       }
     }
-  } else if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
+
+    return;
+  }
+
+  if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
     if(LOG_ENABLED(INFO)) {
       if(events & BEV_EVENT_ERROR) {
         DCLOG(INFO, dconn) << "Network error";

src/shrpx_spdy_upstream.cc

@@ -103,37 +103,41 @@ void on_stream_close_callback
                          << " is being closed";
   }
   auto downstream = upstream->find_downstream(stream_id);
-  if(downstream) {
+  if(!downstream) {
-    if(downstream->get_request_state() == Downstream::CONNECT_FAIL) {
+    return;
-      upstream->remove_downstream(downstream);
+  }
-      delete downstream;
-    } else {
-      downstream->set_request_state(Downstream::STREAM_CLOSED);
-      if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
-        // At this point, downstream response was read
-        if(!downstream->get_upgraded() &&
-           !downstream->get_response_connection_close()) {
-          // Keep-alive
-          auto dconn = downstream->get_downstream_connection();
-          if(dconn) {
-            dconn->detach_downstream(downstream);
-          }
-        }
-        upstream->remove_downstream(downstream);
-        delete downstream;
-      } else {
-        // At this point, downstream read may be paused.
 
-        // If shrpx_downstream::push_request_headers() failed, the
+  if(downstream->get_request_state() == Downstream::CONNECT_FAIL) {
-        // error is handled here.
+    upstream->remove_downstream(downstream);
-        upstream->remove_downstream(downstream);
+    delete downstream;
-        delete downstream;
+    return;
-        // How to test this case? Request sufficient large download
+  }
-        // and make client send RST_STREAM after it gets first DATA
+
-        // frame chunk.
+  downstream->set_request_state(Downstream::STREAM_CLOSED);
+  if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
+    // At this point, downstream response was read
+    if(!downstream->get_upgraded() &&
+       !downstream->get_response_connection_close()) {
+      // Keep-alive
+      auto dconn = downstream->get_downstream_connection();
+      if(dconn) {
+        dconn->detach_downstream(downstream);
       }
     }
+    upstream->remove_downstream(downstream);
+    delete downstream;
+    return;
   }
+
+  // At this point, downstream read may be paused.
+
+  // If shrpx_downstream::push_request_headers() failed, the
+  // error is handled here.
+  upstream->remove_downstream(downstream);
+  delete downstream;
+  // How to test this case? Request sufficient large download
+  // and make client send RST_STREAM after it gets first DATA
+  // frame chunk.
 }
 } // namespace
 
@@ -245,43 +249,49 @@ void on_data_chunk_recv_callback(spdylay_session *session,
 {
   auto upstream = static_cast<SpdyUpstream*>(user_data);
   auto downstream = upstream->find_downstream(stream_id);
-  if(downstream) {
+
-    if(downstream->push_upload_data_chunk(data, len) != 0) {
+  if(!downstream) {
-      upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR);
+    return;
-      return;
+  }
+
+  if(downstream->push_upload_data_chunk(data, len) != 0) {
+    upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR);
+    return;
+  }
+
+  if(!upstream->get_flow_control()) {
+    return;
+  }
+
+  // If connection-level window control is not enabled (e.g,
+  // spdy/3), spdylay_session_get_recv_data_length() is always
+  // returns 0.
+  if(spdylay_session_get_recv_data_length(session) >
+     std::max(SPDYLAY_INITIAL_WINDOW_SIZE,
+              1 << get_config()->http2_upstream_connection_window_bits)) {
+    if(LOG_ENABLED(INFO)) {
+      ULOG(INFO, upstream)
+        << "Flow control error on connection: "
+        << "recv_window_size="
+        << spdylay_session_get_recv_data_length(session)
+        << ", window_size="
+        << (1 << get_config()->http2_upstream_connection_window_bits);
     }
-    if(upstream->get_flow_control()) {
+    spdylay_session_fail_session(session, SPDYLAY_GOAWAY_PROTOCOL_ERROR);
-      // If connection-level window control is not enabled (e.g,
+    return;
-      // spdy/3), spdylay_session_get_recv_data_length() is always
+  }
-      // returns 0.
+  if(spdylay_session_get_stream_recv_data_length(session, stream_id) >
-      if(spdylay_session_get_recv_data_length(session) >
+     std::max(SPDYLAY_INITIAL_WINDOW_SIZE,
-         std::max(SPDYLAY_INITIAL_WINDOW_SIZE,
+              1 << get_config()->http2_upstream_window_bits)) {
-                  1 << get_config()->http2_upstream_connection_window_bits)) {
+    if(LOG_ENABLED(INFO)) {
-        if(LOG_ENABLED(INFO)) {
+      ULOG(INFO, upstream)
-          ULOG(INFO, upstream)
+        << "Flow control error: recv_window_size="
-            << "Flow control error on connection: "
+        << spdylay_session_get_stream_recv_data_length(session, stream_id)
-            << "recv_window_size="
+        << ", initial_window_size="
-            << spdylay_session_get_recv_data_length(session)
+        << (1 << get_config()->http2_upstream_window_bits);
-            << ", window_size="
-            << (1 << get_config()->http2_upstream_connection_window_bits);
-        }
-        spdylay_session_fail_session(session, SPDYLAY_GOAWAY_PROTOCOL_ERROR);
-        return;
-      }
-      if(spdylay_session_get_stream_recv_data_length(session, stream_id) >
-         std::max(SPDYLAY_INITIAL_WINDOW_SIZE,
-                  1 << get_config()->http2_upstream_window_bits)) {
-        if(LOG_ENABLED(INFO)) {
-          ULOG(INFO, upstream)
-            << "Flow control error: recv_window_size="
-            << spdylay_session_get_stream_recv_data_length(session, stream_id)
-            << ", initial_window_size="
-            << (1 << get_config()->http2_upstream_window_bits);
-        }
-        upstream->rst_stream(downstream, SPDYLAY_FLOW_CONTROL_ERROR);
-        return;
-      }
     }
+    upstream->rst_stream(downstream, SPDYLAY_FLOW_CONTROL_ERROR);
+    return;
   }
 }
 } // namespace
@@ -520,11 +530,11 @@ void spdy_downstream_readcb(bufferevent *bev, void *ptr)
     // on_stream_close_callback.
     upstream->rst_stream(downstream, infer_upstream_rst_stream_status_code
                          (downstream->get_response_rst_stream_error_code()));
-    downstream->set_downstream_connection(0);
+    downstream->set_downstream_connection(nullptr);
     delete dconn;
-    dconn = 0;
+    dconn = nullptr;
   } else {
-    int rv = downstream->on_read();
+    auto rv = downstream->on_read();
     if(rv != 0) {
       if(LOG_ENABLED(INFO)) {
         DCLOG(INFO, dconn) << "HTTP parser failure";
@@ -541,9 +551,9 @@ void spdy_downstream_readcb(bufferevent *bev, void *ptr)
       downstream->set_response_state(Downstream::MSG_COMPLETE);
       // Clearly, we have to close downstream connection on http parser
       // failure.
-      downstream->set_downstream_connection(0);
+      downstream->set_downstream_connection(nullptr);
       delete dconn;
-      dconn = 0;
+      dconn = nullptr;
     }
   }
   if(upstream->send() != 0) {
@@ -573,6 +583,7 @@ void spdy_downstream_eventcb(bufferevent *bev, short events, void *ptr)
   auto dconn = static_cast<DownstreamConnection*>(ptr);
   auto downstream = dconn->get_downstream();
   auto upstream = static_cast<SpdyUpstream*>(downstream->get_upstream());
+
   if(events & BEV_EVENT_CONNECTED) {
     if(LOG_ENABLED(INFO)) {
       DCLOG(INFO, dconn) << "Connection established. stream_id="
@@ -585,7 +596,10 @@ void spdy_downstream_eventcb(bufferevent *bev, short events, void *ptr)
       DCLOG(WARNING, dconn) << "Setting option TCP_NODELAY failed: errno="
                             << errno;
     }
-  } else if(events & BEV_EVENT_EOF) {
+    return;
+  }
+
+  if(events & BEV_EVENT_EOF) {
     if(LOG_ENABLED(INFO)) {
       DCLOG(INFO, dconn) << "EOF. stream_id=" << downstream->get_stream_id();
     }
@@ -594,41 +608,46 @@ void spdy_downstream_eventcb(bufferevent *bev, short events, void *ptr)
       // the first place. We can delete downstream.
       upstream->remove_downstream(downstream);
       delete downstream;
-    } else {
+      return;
-      // Delete downstream connection. If we don't delete it here, it
+    }
-      // will be pooled in on_stream_close_callback.
-      downstream->set_downstream_connection(0);
-      delete dconn;
-      dconn = 0;
-      // downstream wil be deleted in on_stream_close_callback.
-      if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
-        // Server may indicate the end of the request by EOF
-        if(LOG_ENABLED(INFO)) {
-          ULOG(INFO, upstream) << "Downstream body was ended by EOF";
-        }
-        downstream->set_response_state(Downstream::MSG_COMPLETE);
 
-        // For tunneled connection, MSG_COMPLETE signals
+    // Delete downstream connection. If we don't delete it here, it
-        // spdy_data_read_callback to send RST_STREAM after pending
+    // will be pooled in on_stream_close_callback.
-        // response body is sent. This is needed to ensure that
+    downstream->set_downstream_connection(nullptr);
-        // RST_STREAM is sent after all pending data are sent.
+    delete dconn;
-        upstream->on_downstream_body_complete(downstream);
+    dconn = nullptr;
-      } else if(downstream->get_response_state() != Downstream::MSG_COMPLETE) {
+    // downstream wil be deleted in on_stream_close_callback.
-        // If stream was not closed, then we set MSG_COMPLETE and let
+    if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
-        // on_stream_close_callback delete downstream.
+      // Server may indicate the end of the request by EOF
-        if(upstream->error_reply(downstream, 502) != 0) {
+      if(LOG_ENABLED(INFO)) {
-          delete upstream->get_client_handler();
+        ULOG(INFO, upstream) << "Downstream body was ended by EOF";
-          return;
-        }
-        downstream->set_response_state(Downstream::MSG_COMPLETE);
       }
-      if(upstream->send() != 0) {
+      downstream->set_response_state(Downstream::MSG_COMPLETE);
+
+      // For tunneled connection, MSG_COMPLETE signals
+      // spdy_data_read_callback to send RST_STREAM after pending
+      // response body is sent. This is needed to ensure that
+      // RST_STREAM is sent after all pending data are sent.
+      upstream->on_downstream_body_complete(downstream);
+    } else if(downstream->get_response_state() != Downstream::MSG_COMPLETE) {
+      // If stream was not closed, then we set MSG_COMPLETE and let
+      // on_stream_close_callback delete downstream.
+      if(upstream->error_reply(downstream, 502) != 0) {
         delete upstream->get_client_handler();
         return;
       }
-      // At this point, downstream may be deleted.
+      downstream->set_response_state(Downstream::MSG_COMPLETE);
     }
-  } else if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
+    if(upstream->send() != 0) {
+      delete upstream->get_client_handler();
+      return;
+    }
+    // At this point, downstream may be deleted.
+
+    return;
+  }
+
+  if(events & (BEV_EVENT_ERROR | BEV_EVENT_TIMEOUT)) {
     if(LOG_ENABLED(INFO)) {
       if(events & BEV_EVENT_ERROR) {
         DCLOG(INFO, dconn) << "Downstream network error: "
@@ -644,42 +663,45 @@ void spdy_downstream_eventcb(bufferevent *bev, short events, void *ptr)
     if(downstream->get_request_state() == Downstream::STREAM_CLOSED) {
       upstream->remove_downstream(downstream);
       delete downstream;
-    } else {
+      return;
-      // Delete downstream connection. If we don't delete it here, it
-      // will be pooled in on_stream_close_callback.
-      downstream->set_downstream_connection(0);
-      delete dconn;
-      dconn = 0;
-      if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
-        // For SSL tunneling, we issue RST_STREAM. For other types of
-        // stream, we don't have to do anything since response was
-        // complete.
-        if(downstream->get_upgraded()) {
-          upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR);
-        }
-      } else {
-        if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
-          upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR);
-        } else {
-          unsigned int status;
-          if(events & BEV_EVENT_TIMEOUT) {
-            status = 504;
-          } else {
-            status = 502;
-          }
-          if(upstream->error_reply(downstream, status) != 0) {
-            delete upstream->get_client_handler();
-            return;
-          }
-        }
-        downstream->set_response_state(Downstream::MSG_COMPLETE);
-      }
-      if(upstream->send() != 0) {
-        delete upstream->get_client_handler();
-        return;
-      }
-      // At this point, downstream may be deleted.
     }
+
+    // Delete downstream connection. If we don't delete it here, it
+    // will be pooled in on_stream_close_callback.
+    downstream->set_downstream_connection(nullptr);
+    delete dconn;
+    dconn = nullptr;
+
+    if(downstream->get_response_state() == Downstream::MSG_COMPLETE) {
+      // For SSL tunneling, we issue RST_STREAM. For other types of
+      // stream, we don't have to do anything since response was
+      // complete.
+      if(downstream->get_upgraded()) {
+        upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR);
+      }
+    } else {
+      if(downstream->get_response_state() == Downstream::HEADER_COMPLETE) {
+        upstream->rst_stream(downstream, SPDYLAY_INTERNAL_ERROR);
+      } else {
+        unsigned int status;
+        if(events & BEV_EVENT_TIMEOUT) {
+          status = 504;
+        } else {
+          status = 502;
+        }
+        if(upstream->error_reply(downstream, status) != 0) {
+          delete upstream->get_client_handler();
+          return;
+        }
+      }
+      downstream->set_response_state(Downstream::MSG_COMPLETE);
+    }
+    if(upstream->send() != 0) {
+      delete upstream->get_client_handler();
+      return;
+    }
+    // At this point, downstream may be deleted.
+    return;
   }
 }
 } // namespace