Replace nghttp2_set_option with nghttp2_session_{client,server}_new2

nghttp2_session_client_new2 and nghttp2_session_server_new2 take
additional parameters which specifies session options.
nghttp2_set_option is somewhat crumsy because of type checking.
Now we use nghttp2_opt_set, which specifies individual options with
types. We changed the value of nghttp2_opt, so this change will
require re-compile.

lib/includes/nghttp2/nghttp2.h

@@ -1236,14 +1236,6 @@ int nghttp2_session_server_new(nghttp2_session **session_ptr,
                                const nghttp2_session_callbacks *callbacks,
                                void *user_data);
 
-/**
- * @function
- *
- * Frees any resources allocated for |session|. If |session| is
- * ``NULL``, this function does nothing.
- */
-void nghttp2_session_del(nghttp2_session *session);
-
 /**
  * @enum
  *
@@ -1252,18 +1244,22 @@ void nghttp2_session_del(nghttp2_session *session);
 typedef enum {
   /**
    * This option prevents the library from sending WINDOW_UPDATE for a
-   * stream automatically. If this option is set, the application is
+   * stream automatically. If this option is set to nonzero, the
-   * responsible for sending WINDOW_UPDATE using
+   * library won't send WINDOW_UPDATE for a stream and the application
-   * `nghttp2_submit_window_update`.
+   * is responsible for sending WINDOW_UPDATE using
+   * `nghttp2_submit_window_update`. By default, this option is set to
+   * zero.
    */
   NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE = 1,
   /**
    * This option prevents the library from sending WINDOW_UPDATE for a
-   * connection automatically. If this option is set, the application
+   * connection automatically. If this option is set to nonzero, the
-   * is responsible for sending WINDOW_UPDATE with stream ID 0 using
+   * library won't send WINDOW_UPDATE for a connection and the
-   * `nghttp2_submit_window_update`.
+   * application is responsible for sending WINDOW_UPDATE with stream
+   * ID 0 using `nghttp2_submit_window_update`. By default, this
+   * option is set to zero.
    */
-  NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE = 2,
+  NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE = 1 << 1,
   /**
    * This option sets the SETTINGS_MAX_CONCURRENT_STREAMS value of
    * remote endpoint as if it is received in SETTINGS frame. Without
@@ -1277,47 +1273,90 @@ typedef enum {
    * will be overwritten if the local endpoint receives
    * SETTINGS_MAX_CONCURRENT_STREAMS from the remote endpoint.
    */
-  NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS = 3
+  NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS = 1 << 2
 } nghttp2_opt;
 
+/**
+ * @struct
+ *
+ * Struct to store option values for nghttp2_session.
+ */
+typedef struct {
+  /**
+   * :enum:`NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE`
+   */
+  uint8_t no_auto_stream_window_update;
+  /**
+   * :enum:`NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE`
+   */
+  uint8_t no_auto_connection_window_update;
+  /**
+   * :enum:`NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS`
+   */
+  uint32_t peer_max_concurrent_streams;
+} nghttp2_opt_set;
+
 /**
  * @function
  *
- * Sets the configuration option for the |session|.  The |optname| is
+ * Like `nghttp2_session_client_new()`, but with additional options
- * one of :type:`nghttp2_opt`. The |optval| is the pointer to the
+ * specified in the |opt_set|. The caller must set bitwise-OR of
- * option value and the |optlen| is the size of |*optval|. The
+ * :enum:`nghttp2_opt` for given options.  For example, if it
- * required type of |optval| varies depending on the |optname|. See
+ * specifies :enum:`NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE` and
- * below.
+ * :enum:`NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS` in the |opt_set|,
+ * the |opt_set_mask| should be
+ * ``NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE |
+ * NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS``.
  *
- * The following |optname| are supported:
+ * If the |opt_set_mask| is 0, the |opt_set| could be ``NULL`` safely
- *
+ * and the call is equivalent to `nghttp2_session_client_new()`.
- * :enum:`NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE`
- *     The |optval| must be a pointer to ``int``. If the |*optval| is
- *     nonzero, the library will not send WINDOW_UPDATE for a stream
- *     automatically.  Therefore, the application is responsible for
- *     sending WINDOW_UPDATE using
- *     `nghttp2_submit_window_update`. This option defaults to 0.
- *
- * :enum:`NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE`
- *     The |optval| must be a pointer to ``int``. If the |*optval| is
- *     nonzero, the library will not send WINDOW_UPDATE for connection
- *     automatically.  Therefore, the application is responsible for
- *     sending WINDOW_UPDATE using
- *     `nghttp2_submit_window_update`. This option defaults to 0.
- *
- * :enum:`NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS`
- *     The |optval| must be a pointer to ``ssize_t``. It is an error
- *     if |*optval| is 0 or negative.
  *
  * This function returns 0 if it succeeds, or one of the following
  * negative error codes:
  *
- * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT`
+ * :enum:`NGHTTP2_ERR_NOMEM`
- *     The |optname| is not supported; or the |optval| and/or the
+ *     Out of memory.
- *     |optlen| are invalid.
  */
-int nghttp2_session_set_option(nghttp2_session *session,
+int nghttp2_session_client_new2(nghttp2_session **session_ptr,
-                               int optname, void *optval, size_t optlen);
+                                const nghttp2_session_callbacks *callbacks,
+                                void *user_data,
+                                uint32_t opt_set_mask,
+                                const nghttp2_opt_set *opt_set);
+
+/**
+ * @function
+ *
+ * Like `nghttp2_session_server_new()`, but with additional options
+ * specified in the |opt_set|. The caller must set bitwise-OR of
+ * :enum:`nghttp2_opt` for given options.  For example, if it
+ * specifies :enum:`NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE` and
+ * :enum:`NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS` in the |opt_set|,
+ * the |opt_set_mask| should be
+ * ``NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE |
+ * NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS``.
+ *
+ * If the |opt_set_mask| is 0, the |opt_set| could be ``NULL`` safely
+ * and the call is equivalent to `nghttp2_session_server_new()`.
+ *
+ * This function returns 0 if it succeeds, or one of the following
+ * negative error codes:
+ *
+ * :enum:`NGHTTP2_ERR_NOMEM`
+ *     Out of memory.
+ */
+int nghttp2_session_server_new2(nghttp2_session **session_ptr,
+                                const nghttp2_session_callbacks *callbacks,
+                                void *user_data,
+                                uint32_t opt_set_mask,
+                                const nghttp2_opt_set *opt_set);
+
+/**
+ * @function
+ *
+ * Frees any resources allocated for |session|. If |session| is
+ * ``NULL``, this function does nothing.
+ */
+void nghttp2_session_del(nghttp2_session *session);
 
 /**
  * @function

lib/nghttp2_session.c

@@ -177,7 +177,9 @@ static void init_settings(uint32_t *settings)
 static int nghttp2_session_new(nghttp2_session **session_ptr,
                                const nghttp2_session_callbacks *callbacks,
                                void *user_data,
-                               int server)
+                               int server,
+                               uint32_t opt_set_mask,
+                               const nghttp2_opt_set *opt_set)
 {
   int r;
   nghttp2_hd_side side_deflate, side_inflate;
@@ -193,6 +195,16 @@ static int nghttp2_session_new(nghttp2_session **session_ptr,
 
   (*session_ptr)->next_seq = 0;
 
+  if((opt_set_mask & NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE) &&
+     opt_set->no_auto_stream_window_update) {
+    (*session_ptr)->opt_flags |= NGHTTP2_OPTMASK_NO_AUTO_STREAM_WINDOW_UPDATE;
+  }
+  if((opt_set_mask & NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE) &&
+     opt_set->no_auto_connection_window_update) {
+    (*session_ptr)->opt_flags |=
+      NGHTTP2_OPTMASK_NO_AUTO_CONNECTION_WINDOW_UPDATE;
+  }
+
   (*session_ptr)->remote_flow_control = 1;
   (*session_ptr)->local_flow_control = 1;
   (*session_ptr)->remote_window_size = NGHTTP2_INITIAL_CONNECTION_WINDOW_SIZE;
@@ -254,6 +266,11 @@ static int nghttp2_session_new(nghttp2_session **session_ptr,
   init_settings((*session_ptr)->remote_settings);
   init_settings((*session_ptr)->local_settings);
 
+  if(opt_set_mask & NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS) {
+    (*session_ptr)->remote_settings[NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS] =
+      opt_set->peer_max_concurrent_streams;
+  }
+
   (*session_ptr)->callbacks = *callbacks;
   (*session_ptr)->user_data = user_data;
 
@@ -290,10 +307,21 @@ static int nghttp2_session_new(nghttp2_session **session_ptr,
 int nghttp2_session_client_new(nghttp2_session **session_ptr,
                                const nghttp2_session_callbacks *callbacks,
                                void *user_data)
+{
+  return nghttp2_session_client_new2(session_ptr, callbacks, user_data,
+                                     0, NULL);
+}
+
+int nghttp2_session_client_new2(nghttp2_session **session_ptr,
+                                const nghttp2_session_callbacks *callbacks,
+                                void *user_data,
+                                uint32_t opt_set_mask,
+                                const nghttp2_opt_set *opt_set)
 {
   int r;
   /* For client side session, header compression is disabled. */
-  r = nghttp2_session_new(session_ptr, callbacks, user_data, 0);
+  r = nghttp2_session_new(session_ptr, callbacks, user_data, 0,
+                          opt_set_mask, opt_set);
   if(r == 0) {
     /* IDs for use in client */
     (*session_ptr)->next_stream_id = 1;
@@ -305,10 +333,21 @@ int nghttp2_session_client_new(nghttp2_session **session_ptr,
 int nghttp2_session_server_new(nghttp2_session **session_ptr,
                                const nghttp2_session_callbacks *callbacks,
                                void *user_data)
+{
+  return nghttp2_session_server_new2(session_ptr, callbacks, user_data,
+                                     0, NULL);
+}
+
+int nghttp2_session_server_new2(nghttp2_session **session_ptr,
+                                const nghttp2_session_callbacks *callbacks,
+                                void *user_data,
+                                uint32_t opt_set_mask,
+                                const nghttp2_opt_set *opt_set)
 {
   int r;
   /* Enable header compression on server side. */
-  r = nghttp2_session_new(session_ptr, callbacks, user_data, 1);
+  r = nghttp2_session_new(session_ptr, callbacks, user_data, 1,
+                          opt_set_mask, opt_set);
   if(r == 0) {
     /* IDs for use in client */
     (*session_ptr)->next_stream_id = 2;
@@ -3626,49 +3665,6 @@ int32_t nghttp2_session_get_effective_local_window_size
   return session->local_window_size;
 }
 
-int nghttp2_session_set_option(nghttp2_session *session,
-                               int optname, void *optval, size_t optlen)
-{
-  switch(optname) {
-  case NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE:
-  case NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE: {
-    int flag;
-    if(optname == NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE) {
-      flag = NGHTTP2_OPTMASK_NO_AUTO_STREAM_WINDOW_UPDATE;
-    } else {
-      flag = NGHTTP2_OPTMASK_NO_AUTO_CONNECTION_WINDOW_UPDATE;
-    }
-    if(optlen == sizeof(int)) {
-      int intval = *(int*)optval;
-      if(intval) {
-        session->opt_flags |= flag;
-      } else {
-        session->opt_flags &= ~flag;
-      }
-    } else {
-      return NGHTTP2_ERR_INVALID_ARGUMENT;
-    }
-    break;
-  }
-  case NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS: {
-    ssize_t sszval;
-    if(optlen != sizeof(ssize_t)) {
-      return NGHTTP2_ERR_INVALID_ARGUMENT;
-    }
-    sszval = *(ssize_t*)optval;
-    if(sszval <= 0) {
-      return NGHTTP2_ERR_INVALID_ARGUMENT;
-    }
-    session->remote_settings[NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS] =
-      sszval;
-    break;
-  }
-  default:
-    return NGHTTP2_ERR_INVALID_ARGUMENT;
-  }
-  return 0;
-}
-
 int nghttp2_session_upgrade(nghttp2_session *session,
                             const uint8_t *settings_payload,
                             size_t settings_payloadlen,

src/nghttp.cc

@@ -665,14 +665,11 @@ struct HttpClient {
     if(!need_upgrade()) {
       record_handshake_time();
     }
-    rv = nghttp2_session_client_new(&session, callbacks, this);
+    nghttp2_opt_set opt_set;
-    if(rv != 0) {
+    opt_set.peer_max_concurrent_streams = config.peer_max_concurrent_streams;
-      return -1;
+    rv = nghttp2_session_client_new2(&session, callbacks, this,
-    }
-    rv = nghttp2_session_set_option(session,
                                      NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS,
-                                    &config.peer_max_concurrent_streams,
+                                     &opt_set);
-                                    sizeof(config.peer_max_concurrent_streams));
     if(rv != 0) {
       return -1;
     }

src/shrpx_http2_session.cc

@@ -1148,21 +1148,19 @@ int Http2Session::on_connect()
     on_frame_recv_parse_error_callback;
   callbacks.on_unknown_frame_recv_callback = on_unknown_frame_recv_callback;
 
-  rv = nghttp2_session_client_new(&session_, &callbacks, this);
+  nghttp2_opt_set opt_set;
+  opt_set.no_auto_stream_window_update = 1;
+  opt_set.no_auto_connection_window_update = 1;
+  uint32_t opt_set_mask =
+    NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE |
+    NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE;
+  rv = nghttp2_session_client_new2(&session_, &callbacks, this,
+                                   opt_set_mask, &opt_set);
   if(rv != 0) {
     return -1;
   }
 
-  int val = 1;
   flow_control_ = true;
-  rv = nghttp2_session_set_option(session_,
-                                  NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE,
-                                  &val, sizeof(val));
-  assert(rv == 0);
-  rv = nghttp2_session_set_option(session_,
-                                  NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE,
-                                  &val, sizeof(val));
-  assert(rv == 0);
 
   nghttp2_settings_entry entry[3];
   entry[0].settings_id = NGHTTP2_SETTINGS_ENABLE_PUSH;

src/shrpx_http2_upstream.cc

@@ -484,21 +484,19 @@ Http2Upstream::Http2Upstream(ClientHandler *handler)
     on_frame_recv_parse_error_callback;
   callbacks.on_unknown_frame_recv_callback = on_unknown_frame_recv_callback;
 
+  nghttp2_opt_set opt_set;
+  opt_set.no_auto_stream_window_update = 1;
+  opt_set.no_auto_connection_window_update = 1;
+  uint32_t opt_set_mask =
+    NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE |
+    NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE;
   int rv;
-  rv = nghttp2_session_server_new(&session_, &callbacks, this);
+  rv = nghttp2_session_server_new2(&session_, &callbacks, this,
+                                   opt_set_mask, &opt_set);
   assert(rv == 0);
 
-  int val = 1;
   flow_control_ = true;
   initial_window_size_ = (1 << get_config()->http2_upstream_window_bits) - 1;
-  rv = nghttp2_session_set_option(session_,
-                                  NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE,
-                                  &val, sizeof(val));
-  assert(rv == 0);
-  rv = nghttp2_session_set_option(session_,
-                                  NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE,
-                                  &val, sizeof(val));
-  assert(rv == 0);
 
   // TODO Maybe call from outside?
   nghttp2_settings_entry entry[2];

tests/nghttp2_session_test.c

@@ -3605,73 +3605,36 @@ void test_nghttp2_session_set_option(void)
 {
   nghttp2_session* session;
   nghttp2_session_callbacks callbacks;
-  int intval;
+  nghttp2_opt_set opt_set;
-  char charval;
+
-  ssize_t sszval;
+  opt_set.no_auto_stream_window_update = 1;
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
-  nghttp2_session_client_new(&session, &callbacks, NULL);
+  nghttp2_session_client_new2(&session, &callbacks, NULL,
-
-  intval = 1;
-  CU_ASSERT(0 ==
-            nghttp2_session_set_option
-            (session,
                               NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE,
-             &intval, sizeof(intval)));
+                              &opt_set);
   CU_ASSERT(session->opt_flags & NGHTTP2_OPTMASK_NO_AUTO_STREAM_WINDOW_UPDATE);
+  CU_ASSERT(!(session->opt_flags &
+              NGHTTP2_OPTMASK_NO_AUTO_CONNECTION_WINDOW_UPDATE));
+  nghttp2_session_del(session);
 
-  intval = 0;
+  opt_set.no_auto_stream_window_update = 0;
-  CU_ASSERT(0 ==
+  opt_set.no_auto_connection_window_update = 1;
-            nghttp2_session_set_option
+  nghttp2_session_server_new2(&session, &callbacks, NULL,
-            (session,
-             NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE,
-             &intval, sizeof(intval)));
-  CU_ASSERT((session->opt_flags &
-             NGHTTP2_OPTMASK_NO_AUTO_STREAM_WINDOW_UPDATE) == 0);
-
-  CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT ==
-            nghttp2_session_set_option(session, 0, /* 0 is invalid optname */
-                                       &intval, sizeof(intval)));
-
-  charval = 1;
-  CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT ==
-            nghttp2_session_set_option
-            (session,
-             NGHTTP2_OPT_NO_AUTO_STREAM_WINDOW_UPDATE,
-             &charval, sizeof(charval)));
-
-  intval = 1;
-  CU_ASSERT(0 ==
-            nghttp2_session_set_option
-            (session,
                               NGHTTP2_OPT_NO_AUTO_CONNECTION_WINDOW_UPDATE,
-             &intval, sizeof(intval)));
+                              &opt_set);
+  CU_ASSERT(!(session->opt_flags &
+              NGHTTP2_OPTMASK_NO_AUTO_STREAM_WINDOW_UPDATE));
   CU_ASSERT(session->opt_flags &
             NGHTTP2_OPTMASK_NO_AUTO_CONNECTION_WINDOW_UPDATE);
+  nghttp2_session_del(session);
 
-  sszval = 100;
+  opt_set.peer_max_concurrent_streams = 100;
-  CU_ASSERT(0 ==
+  nghttp2_session_client_new2(&session, &callbacks, NULL,
-            nghttp2_session_set_option
-            (session,
                               NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS,
-             &sszval, sizeof(sszval)));
+                              &opt_set);
-  CU_ASSERT(sszval ==
+  CU_ASSERT(100 ==
-            (ssize_t)session->
+            session->
             remote_settings[NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS]);
-
-  sszval = 0;
-  CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT ==
-            nghttp2_session_set_option
-            (session,
-             NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS,
-             &sszval, sizeof(sszval)));
-
-  charval = 100;
-  CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT ==
-            nghttp2_session_set_option
-            (session,
-             NGHTTP2_OPT_PEER_MAX_CONCURRENT_STREAMS,
-             &charval, sizeof(charval)));
-
   nghttp2_session_del(session);
 }