Merge pull request #1775 from nghttp2/src-eliminate-strtoul

Replace the use of strtoul and strtol with parse_uint

src/CMakeLists.txt

@@ -235,6 +235,8 @@ if(ENABLE_HPACK_TOOLS)
   set(deflatehd_SOURCES
     deflatehd.cc
     comp_helper.c
+    util.cc
+    timegm.c
   )
   add_executable(inflatehd ${inflatehd_SOURCES})
   add_executable(deflatehd ${deflatehd_SOURCES})

src/Makefile.am

@@ -246,7 +246,10 @@ if ENABLE_HPACK_TOOLS
 
 bin_PROGRAMS += inflatehd deflatehd
 
-HPACK_TOOLS_COMMON_SRCS = comp_helper.c comp_helper.h
+HPACK_TOOLS_COMMON_SRCS = \
+	comp_helper.c comp_helper.h \
+	util.cc util.h \
+	timegm.c timegm.h
 
 inflatehd_SOURCES = inflatehd.cc $(HPACK_TOOLS_COMMON_SRCS)
 

src/deflatehd.cc

@@ -45,6 +45,7 @@
 
 #include "template.h"
 #include "comp_helper.h"
+#include "util.h"
 
 namespace nghttp2 {
 
@@ -381,8 +382,6 @@ constexpr static struct option long_options[] = {
     {nullptr, 0, nullptr, 0}};
 
 int main(int argc, char **argv) {
-  char *end;
-
   config.table_size = 4_k;
   config.deflate_table_size = 4_k;
   config.http1text = 0;
@@ -401,24 +400,26 @@ int main(int argc, char **argv) {
       // --http1text
       config.http1text = 1;
       break;
-    case 's':
+    case 's': {
       // --table-size
-      errno = 0;
+      auto n = util::parse_uint(optarg);
-      config.table_size = strtoul(optarg, &end, 10);
+      if (n == -1) {
-      if (errno == ERANGE || *end != '\0') {
         fprintf(stderr, "-s: Bad option value\n");
         exit(EXIT_FAILURE);
       }
+      config.table_size = n;
       break;
-    case 'S':
+    }
+    case 'S': {
       // --deflate-table-size
-      errno = 0;
+      auto n = util::parse_uint(optarg);
-      config.deflate_table_size = strtoul(optarg, &end, 10);
+      if (n == -1) {
-      if (errno == ERANGE || *end != '\0') {
         fprintf(stderr, "-S: Bad option value\n");
         exit(EXIT_FAILURE);
       }
+      config.deflate_table_size = n;
       break;
+    }
     case 'd':
       // --dump-header-table
       config.dump_header_table = 1;

src/h2load.cc

@@ -2370,44 +2370,65 @@ int main(int argc, char **argv) {
       break;
     }
     switch (c) {
-    case 'n':
+    case 'n': {
-      config.nreqs = strtoul(optarg, nullptr, 10);
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-n: bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.nreqs = n;
       nreqs_set_manually = true;
       break;
-    case 'c':
+    }
-      config.nclients = strtoul(optarg, nullptr, 10);
+    case 'c': {
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-c: bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.nclients = n;
       break;
+    }
     case 'd':
       datafile = optarg;
       break;
-    case 't':
+    case 't': {
 #ifdef NOTHREADS
       std::cerr << "-t: WARNING: Threading disabled at build time, "
                 << "no threads created." << std::endl;
 #else
-      config.nthreads = strtoul(optarg, nullptr, 10);
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-t: bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.nthreads = n;
 #endif // NOTHREADS
       break;
-    case 'm':
+    }
-      config.max_concurrent_streams = strtoul(optarg, nullptr, 10);
+    case 'm': {
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-m: bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.max_concurrent_streams = n;
       break;
+    }
     case 'w':
     case 'W': {
-      errno = 0;
+      auto n = util::parse_uint(optarg);
-      char *endptr = nullptr;
+      if (n == -1 || n > 30) {
-      auto n = strtoul(optarg, &endptr, 10);
-      if (errno == 0 && *endptr == '\0' && n < 31) {
-        if (c == 'w') {
-          config.window_bits = n;
-        } else {
-          config.connection_window_bits = n;
-        }
-      } else {
         std::cerr << "-" << static_cast<char>(c)
                   << ": specify the integer in the range [0, 30], inclusive"
                   << std::endl;
         exit(EXIT_FAILURE);
       }
+      if (c == 'w') {
+        config.window_bits = n;
+      } else {
+        config.connection_window_bits = n;
+      }
       break;
     }
     case 'f': {
@@ -2470,14 +2491,20 @@ int main(int argc, char **argv) {
       }
       break;
     }
-    case 'r':
+    case 'r': {
-      config.rate = strtoul(optarg, nullptr, 10);
+      auto n = util::parse_uint(optarg);
-      if (config.rate == 0) {
+      if (n == -1) {
+        std::cerr << "-r: bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      if (n == 0) {
         std::cerr << "-r: the rate at which connections are made "
                   << "must be positive." << std::endl;
         exit(EXIT_FAILURE);
       }
+      config.rate = n;
       break;
+    }
     case 'T':
       config.conn_active_timeout = util::parse_duration_with_unit(optarg);
       if (!std::isfinite(config.conn_active_timeout)) {

src/nghttp.cc

@@ -2832,33 +2832,43 @@ int main(int argc, char **argv) {
       break;
     }
     switch (c) {
-    case 'M':
+    case 'M': {
       // peer-max-concurrent-streams option
-      config.peer_max_concurrent_streams = strtoul(optarg, nullptr, 10);
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-M: Bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.peer_max_concurrent_streams = n;
       break;
+    }
     case 'O':
       config.remote_name = true;
       break;
     case 'h':
       print_help(std::cout);
       exit(EXIT_SUCCESS);
-    case 'b':
+    case 'b': {
-      config.padding = strtol(optarg, nullptr, 10);
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-b: Bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.padding = n;
       break;
+    }
     case 'n':
       config.null_out = true;
       break;
     case 'p': {
-      errno = 0;
+      auto n = util::parse_uint(optarg);
-      auto n = strtoul(optarg, nullptr, 10);
+      if (n == -1 || NGHTTP2_MIN_WEIGHT > n || n > NGHTTP2_MAX_WEIGHT) {
-      if (errno == 0 && NGHTTP2_MIN_WEIGHT <= n && n <= NGHTTP2_MAX_WEIGHT) {
-        config.weight.push_back(n);
-      } else {
         std::cerr << "-p: specify the integer in the range ["
                   << NGHTTP2_MIN_WEIGHT << ", " << NGHTTP2_MAX_WEIGHT
                   << "], inclusive" << std::endl;
         exit(EXIT_FAILURE);
       }
+      config.weight.push_back(n);
       break;
     }
     case 'r':
@@ -2884,21 +2894,18 @@ int main(int argc, char **argv) {
       break;
     case 'w':
     case 'W': {
-      errno = 0;
+      auto n = util::parse_uint(optarg);
-      char *endptr = nullptr;
+      if (n == -1 || n > 30) {
-      unsigned long int n = strtoul(optarg, &endptr, 10);
-      if (errno == 0 && *endptr == '\0' && n < 31) {
-        if (c == 'w') {
-          config.window_bits = n;
-        } else {
-          config.connection_window_bits = n;
-        }
-      } else {
         std::cerr << "-" << static_cast<char>(c)
                   << ": specify the integer in the range [0, 30], inclusive"
                   << std::endl;
         exit(EXIT_FAILURE);
       }
+      if (c == 'w') {
+        config.window_bits = n;
+      } else {
+        config.connection_window_bits = n;
+      }
       break;
     }
     case 'H': {
@@ -2939,9 +2946,15 @@ int main(int argc, char **argv) {
     case 'd':
       config.datafile = optarg;
       break;
-    case 'm':
+    case 'm': {
-      config.multiply = strtoul(optarg, nullptr, 10);
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-m: Bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.multiply = n;
       break;
+    }
     case 'c': {
       auto n = util::parse_uint_with_unit(optarg);
       if (n == -1) {
@@ -3025,10 +3038,17 @@ int main(int argc, char **argv) {
         // no-push option
         config.no_push = true;
         break;
-      case 12:
+      case 12: {
         // max-concurrent-streams option
-        config.max_concurrent_streams = strtoul(optarg, nullptr, 10);
+        auto n = util::parse_uint(optarg);
+        if (n == -1) {
+          std::cerr << "--max-concurrent-streams: Bad option value: " << optarg
+                    << std::endl;
+          exit(EXIT_FAILURE);
+        }
+        config.max_concurrent_streams = n;
         break;
+      }
       case 13:
         // expect-continue option
         config.expect_continue = true;

src/nghttpd.cc

@@ -250,9 +250,15 @@ int main(int argc, char **argv) {
     case 'V':
       config.verify_client = true;
       break;
-    case 'b':
+    case 'b': {
-      config.padding = strtol(optarg, nullptr, 10);
+      auto n = util::parse_uint(optarg);
+      if (n == -1) {
+        std::cerr << "-b: Bad option value: " << optarg << std::endl;
+        exit(EXIT_FAILURE);
+      }
+      config.padding = n;
       break;
+    }
     case 'd':
       config.htdocs = optarg;
       break;
@@ -274,13 +280,12 @@ int main(int argc, char **argv) {
       std::cerr << "-n: WARNING: Threading disabled at build time, "
                 << "no threads created." << std::endl;
 #else
-      char *end;
+      auto n = util::parse_uint(optarg);
-      errno = 0;
+      if (n == -1) {
-      config.num_worker = strtoul(optarg, &end, 10);
-      if (errno == ERANGE || *end != '\0' || config.num_worker == 0) {
         std::cerr << "-n: Bad option value: " << optarg << std::endl;
         exit(EXIT_FAILURE);
       }
+      config.num_worker = n;
 #endif // NOTHREADS
       break;
     }
@@ -311,10 +316,8 @@ int main(int argc, char **argv) {
       break;
     case 'w':
     case 'W': {
-      char *endptr;
+      auto n = util::parse_uint(optarg);
-      errno = 0;
+      if (n == -1 || n > 30) {
-      auto n = strtoul(optarg, &endptr, 10);
-      if (errno != 0 || *endptr != '\0' || n >= 31) {
         std::cerr << "-" << static_cast<char>(c)
                   << ": specify the integer in the range [0, 30], inclusive"
                   << std::endl;
@@ -432,7 +435,15 @@ int main(int argc, char **argv) {
     exit(EXIT_FAILURE);
   }
 
-  config.port = strtol(argv[optind++], nullptr, 10);
+  {
+    auto portStr = argv[optind++];
+    auto n = util::parse_uint(portStr);
+    if (n == -1 || n > std::numeric_limits<uint16_t>::max()) {
+      std::cerr << "<PORT>: Bad value: " << portStr << std::endl;
+      exit(EXIT_FAILURE);
+    }
+    config.port = n;
+  }
 
   if (!config.no_tls) {
     config.private_key_file = argv[optind++];

src/shrpx_config.cc

@@ -423,26 +423,6 @@ int parse_uint_with_unit(T *dest, const StringRef &opt,
 }
 } // namespace
 
-// Parses |optarg| as signed integer.  This requires |optarg| to be
-// NULL-terminated string.
-template <typename T>
-int parse_int(T *dest, const StringRef &opt, const char *optarg) {
-  char *end = nullptr;
-
-  errno = 0;
-
-  auto val = strtol(optarg, &end, 10);
-
-  if (!optarg[0] || errno != 0 || *end) {
-    LOG(ERROR) << opt << ": bad value.  Specify an integer.";
-    return -1;
-  }
-
-  *dest = val;
-
-  return 0;
-}
-
 namespace {
 int parse_altsvc(AltSvc &altsvc, const StringRef &opt,
                  const StringRef &optarg) {