common/strtol: fix strict_strtoll() so it accepts hex starting with 0x

- simplify the error handling of strtoll().
- only instantiate ostringstream when handling errors.

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/common/strtol.cc b/src/common/strtol.cc
index b2a90963dff13056f0497c1fad90fcbde3783a76..7f7e78bfbd0db9b7b30acea977a3d9bb76f51bbf 100644 (file)
--- a/src/common/strtol.cc
+++ b/src/common/strtol.cc
@@ -24,27 +24,17 @@ using std::ostringstream;
 
 long long strict_strtoll(const std::string_view str, int base, std::string *err)
 {
-  ostringstream errStr;
-  if (auto invalid = str.find_first_not_of("0123456789-+");
-      invalid != std::string_view::npos ||
-      invalid == 0) {
-    errStr << "The option value '" << str << "' contains invalid digits";
-    *err =  errStr.str();
-    return 0;
-  }
   char *endptr;
   errno = 0; /* To distinguish success/failure after call (see man page) */
   long long ret = strtoll(str.data(), &endptr, base);
-
-  if (endptr == str.data()) {
-    errStr << "Expected option value to be integer, got '" << str << "'";
-    *err =  errStr.str();
+  if (endptr == str.data() || endptr != str.data() + str.size()) {
+    *err = (std::string{"Expected option value to be integer, got '"} +
+           std::string{str} + "'");
     return 0;
   }
-  if ((errno == ERANGE && (ret == LLONG_MAX || ret == LLONG_MIN))
-      || (errno != 0 && ret == 0)) {
-    errStr << "The option value '" << str << "' seems to be invalid";
-    *err = errStr.str();
+  if (errno) {
+    *err = (std::string{"The option value '"} + std::string{str} +
+           "' seems to be invalid");
     return 0;
   }
   *err = "";
@@ -58,11 +48,11 @@ long long strict_strtoll(const char *str, int base, std::string *err)
 
 int strict_strtol(const std::string_view str, int base, std::string *err)
 {
-  ostringstream errStr;
   long long ret = strict_strtoll(str, base, err);
   if (!err->empty())
     return 0;
   if ((ret < INT_MIN) || (ret > INT_MAX)) {
+    ostringstream errStr;
     errStr << "The option value '" << str << "' seems to be invalid";
     *err = errStr.str();
     return 0;
@@ -78,21 +68,23 @@ int strict_strtol(const char *str, int base, std::string *err)
 double strict_strtod(const std::string_view str, std::string *err)
 {
   char *endptr;
-  ostringstream oss;
   errno = 0; /* To distinguish success/failure after call (see man page) */
   double ret = strtod(str.data(), &endptr);
   if (errno == ERANGE) {
+    ostringstream oss;
     oss << "strict_strtod: floating point overflow or underflow parsing '"
        << str << "'";
     *err = oss.str();
     return 0.0;
   }
   if (endptr == str) {
+    ostringstream oss;
     oss << "strict_strtod: expected double, got: '" << str << "'";
     *err = oss.str();
     return 0;
   }
   if (*endptr != '\0') {
+    ostringstream oss;
     oss << "strict_strtod: garbage at end of string. got: '" << str << "'";
     *err = oss.str();
     return 0;
@@ -109,21 +101,23 @@ double strict_strtod(const char *str, std::string *err)
 float strict_strtof(const std::string_view str, std::string *err)
 {
   char *endptr;
-  ostringstream oss;
   errno = 0; /* To distinguish success/failure after call (see man page) */
   float ret = strtof(str.data(), &endptr);
   if (errno == ERANGE) {
+    ostringstream oss;
     oss << "strict_strtof: floating point overflow or underflow parsing '"
        << str << "'";
     *err = oss.str();
     return 0.0;
   }
   if (endptr == str) {
+    ostringstream oss;
     oss << "strict_strtof: expected float, got: '" << str << "'";
     *err = oss.str();
     return 0;
   }
   if (*endptr != '\0') {
+    ostringstream oss;
     oss << "strict_strtof: garbage at end of string. got: '" << str << "'";
     *err = oss.str();
     return 0;

diff --git a/src/test/cli/monmaptool/feature-set-unset-list.t b/src/test/cli/monmaptool/feature-set-unset-list.t
index f574b4318f990a568fcc71e0b69654748f15fc99..216566ebb48a05575207e87a59fdd04fcda9e9f0 100644 (file)
--- a/src/test/cli/monmaptool/feature-set-unset-list.t
+++ b/src/test/cli/monmaptool/feature-set-unset-list.t
@@ -28,7 +28,7 @@
   available:persistent:[kraken(1),luminous(2),mimic(4),osdmap-prune(8)]
 
   $ monmaptool --feature-set foo /tmp/test.monmap.1234
-  unknown features name 'foo' or unable to parse value: The option value 'foo' contains invalid digits
+  unknown features name 'foo' or unable to parse value: Expected option value to be integer, got 'foo'
    usage: [--print] [--create [--clobber][--fsid uuid]]
           [--generate] [--set-initial-members]
           [--add name 1.2.3.4:567] [--rm name]