Handle bad integer args gracefully (#906)

Replace uses of std::stoi with std::sscanf.
When parsing fails, std::stoi throws an invalid-argument exception,
which is not caught.  The standard library then terminates with an abort
about the uncaught exception.  That's unhelpful to users.

Instead, handle bad user input gracefully (without using exceptions)
by using std::sscanf or sscanf_s (on Microsoft runtimes).

1 files changed, 52 insertions, 16 deletions

diff --git a/samples/amber.cc b/samples/amber.cc
index 50bf253..0ea0e00 100644
--- a/samples/amber.cc
+++ b/samples/amber.cc
@@ -14,13 +14,16 @@
 
 #include "amber/amber.h"
 
+#include <stdio.h>
 #include <algorithm>
 #include <cassert>
+#include <cstdio>
 #include <cstdlib>
 #include <fstream>
 #include <iomanip>
 #include <iostream>
 #include <set>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -102,6 +105,29 @@ const char kUsage[] = R"(Usage: amber [options] SCRIPT [SCRIPTS...]
   -h                        -- This help text.
 )";
 
+// Parses a decimal integer from the given string, and writes it to |retval|.
+// Returns true if parsing succeeded and consumed the whole string.
+static bool ParseOneInt(const char* str, int* retval) {
+  char trailing = 0;
+#if defined(_MSC_VER)
+  return sscanf_s(str, "%d%c", retval, &trailing, 1) == 1;
+#else
+  return std::sscanf(str, "%d%c", retval, &trailing) == 1;
+#endif
+}
+
+// Parses a decimal integer, then a period (.), then a decimal integer  from the
+// given string, and writes it to |retval|.  Returns true if parsing succeeded
+// and consumed the whole string.
+static int ParseIntDotInt(const char* str, int* retval0, int* retval1) {
+  char trailing = 0;
+#if defined(_MSC_VER)
+  return sscanf_s(str, "%d.%d%c", retval0, retval1, &trailing, 1) == 2;
+#else
+  return std::sscanf(str, "%d.%d%c", retval0, retval1, &trailing) == 2;
+#endif
+}
+
 bool ParseArgs(const std::vector<std::string>& args, Options* opts) {
   for (size_t i = 1; i < args.size(); ++i) {
     const std::string& arg = args[i];
@@ -168,7 +194,11 @@ bool ParseArgs(const std::vector<std::string>& args, Options* opts) {
         return false;
       }
 
-      int32_t val = std::stoi(std::string(args[i]));
+      int32_t val = 0;
+      if (!ParseOneInt(args[i].c_str(), &val)) {
+        std::cerr << "Invalid device ID: " << args[i] << std::endl;
+        return false;
+      }
       if (val < 0) {
         std::cerr << "Device ID must be non-negative" << std::endl;
         return false;
@@ -182,7 +212,11 @@ bool ParseArgs(const std::vector<std::string>& args, Options* opts) {
         return false;
       }
 
-      int32_t val = std::stoi(std::string(args[i]));
+      int32_t val = 0;
+      if (!ParseOneInt(args[i].c_str(), &val)) {
+        std::cerr << "Invalid fence timeout: " << args[i] << std::endl;
+        return false;
+      }
       if (val < 0) {
         std::cerr << "Fence timeout must be non-negative" << std::endl;
         return false;
@@ -204,23 +238,25 @@ bool ParseArgs(const std::vector<std::string>& args, Options* opts) {
         std::cerr << "Missing value for -v argument." << std::endl;
         return false;
       }
-      const std::string& ver = args[i];
-
-      size_t dot_pos = 0;
-      int32_t val = std::stoi(ver, &dot_pos);
-      if (val < 0) {
-        std::cerr << "Version major must be non-negative" << std::endl;
-        return false;
-      }
-
-      opts->engine_major = static_cast<uint32_t>(val);
-      if (dot_pos != std::string::npos && (dot_pos + 1) < ver.size()) {
-        val = std::stoi(ver.substr(dot_pos + 1));
-        if (val < 0) {
+      const std::string& ver = std::string(args[i]);
+
+      int32_t major = 0;
+      int32_t minor = 0;
+      if (ParseIntDotInt(ver.c_str(), &major, &minor) ||
+          ParseOneInt(ver.c_str(), &major)) {
+        if (major < 0) {
+          std::cerr << "Version major must be non-negative" << std::endl;
+          return false;
+        }
+        if (minor < 0) {
           std::cerr << "Version minor must be non-negative" << std::endl;
           return false;
         }
-        opts->engine_minor = static_cast<uint32_t>(val);
+        opts->engine_major = static_cast<uint32_t>(major);
+        opts->engine_minor = static_cast<uint32_t>(minor);
+      } else {
+        std::cerr << "Invalid engine version number: " << ver << std::endl;
+        return false;
       }
     } else if (arg == "-V" || arg == "--version") {
       opts->show_version_info = true;