You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2018/01/25 16:19:42 UTC

mesos git commit: Made `numify` less dependent on `boost::lexical_cast` failure behavior.

Repository: mesos
Updated Branches:
  refs/heads/master e932ed591 -> b2228a2f3


Made `numify` less dependent on `boost::lexical_cast` failure behavior.

Previously we relied on Boost to never successfully parse hexadecimal
numbers, and only performed rejection of hexadecimal floating point
numbers before entering our custom parser for hexadecimal numbers.
This was not reliable as on some platforms `boost::lexical_cast` ended
up successfully parsing literals of hexadecimal floating point numbers
leading us to never rejecting hexadecimal floating point numbers.

This patch moves the rejection of hexadecimal floating point numbers
to a point before attempting a parse with `boost::lexical_cast`. While
this decouples us in some regards from Boost's parsing behavior, we
also might end up performing more work than needed. Should this become
a concern we should revisit and optimize the implementation here.

Review: https://reviews.apache.org/r/65322/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b2228a2f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b2228a2f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b2228a2f

Branch: refs/heads/master
Commit: b2228a2f323f1239b0687911787f9dfea6d8f1f4
Parents: e932ed5
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Thu Jan 25 17:07:33 2018 +0100
Committer: Benjamin Bannier <bb...@apache.org>
Committed: Thu Jan 25 17:09:14 2018 +0100

----------------------------------------------------------------------
 3rdparty/stout/include/stout/numify.hpp | 77 ++++++++++++++++------------
 1 file changed, 43 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/b2228a2f/3rdparty/stout/include/stout/numify.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/numify.hpp b/3rdparty/stout/include/stout/numify.hpp
index 6db9a78..ecf4fcf 100644
--- a/3rdparty/stout/include/stout/numify.hpp
+++ b/3rdparty/stout/include/stout/numify.hpp
@@ -28,47 +28,56 @@
 template <typename T>
 Try<T> numify(const std::string& s)
 {
+  // Since even with a `0x` prefix `boost::lexical_cast` cannot always
+  // cast all hexadecimal numbers on all platforms (parsing of some
+  // floating point literals seems to work e.g., on macOS, but fails on
+  // some Linuxes), we have to workaround this issue here. We also
+  // process negative hexadecimal numbers `-0x` here to keep it
+  // consistent with non-hexadecimal numbers.
+  bool maybeHex = false;
+
+  if (strings::startsWith(s, "0x") || strings::startsWith(s, "0X") ||
+      strings::startsWith(s, "-0x") || strings::startsWith(s, "-0X")) {
+    maybeHex = true;
+
+    // We disallow hexadecimal floating point numbers.
+    if (strings::contains(s, ".") || strings::contains(s, "p")) {
+      return Error("Failed to convert '" + s + "' to number");
+    }
+  }
+
   try {
     return boost::lexical_cast<T>(s);
   } catch (const boost::bad_lexical_cast&) {
-    // Unfortunately boost::lexical_cast cannot cast a hexadecimal
-    // number even with a "0x" prefix, we have to workaround this
-    // issue here. We also process negative hexadecimal number "-0x"
-    // here to keep it consistent with non-hexadecimal numbers.
-    if (strings::startsWith(s, "0x") || strings::startsWith(s, "0X") ||
-        strings::startsWith(s, "-0x") || strings::startsWith(s, "-0X")) {
-      // NOTE: Hexadecimal floating-point constants (e.g., 0x1p-5,
-      // 0x10.0), are allowed in C99, but cannot be used as floating
-      // point literals in standard C++. Some C++ compilers might
-      // accept them as an extension; for consistency, we always
-      // disallow them.  See:
-      // https://gcc.gnu.org/onlinedocs/gcc/Hex-Floats.html
-      if (!strings::contains(s, ".") && !strings::contains(s, "p")) {
-        T result;
-        std::stringstream ss;
-        // Process negative hexadecimal numbers.
-        if (strings::startsWith(s, "-")) {
-          ss << std::hex << s.substr(1);
-          ss >> result;
-          // Note: When numify is instantiated with unsigned scalars
-          // the expected behaviour is as follow:
-          // numify<T>("-1") == std::numeric_limits<T>::max();
-          // Disabled unary negation warning for all types.
+    // Try to parse hexadecimal numbers by hand if `boost::lexical_cast` failed.
+    if (maybeHex) {
+      T result;
+      std::stringstream ss;
+      // Process negative hexadecimal numbers.
+      if (strings::startsWith(s, "-")) {
+        ss << std::hex << s.substr(1);
+        ss >> result;
+        // NOTE: Negating `result` is safe even if `T` is an unsigned
+        // integer type, because the C++ standard defines the result
+        // to be modulo 2^n in that case.
+        //
+        //     numify<T>("-1") == std::numeric_limits<T>::max();
+        //
+        // Disabled unary negation warning for all types.
 #ifdef __WINDOWS__
-          #pragma warning(disable:4146)
+#pragma warning(disable:4146)
 #endif
-          result = -result;
+        result = -result;
 #ifdef __WINDOWS__
-          #pragma warning(default:4146)
+#pragma warning(default:4146)
 #endif
-        } else {
-          ss << std::hex << s;
-          ss >> result;
-        }
-        // Make sure we really hit the end of the string.
-        if (!ss.fail() && ss.eof()) {
-          return result;
-        }
+      } else {
+        ss << std::hex << s;
+        ss >> result;
+      }
+      // Make sure we really hit the end of the string.
+      if (!ss.fail() && ss.eof()) {
+        return result;
       }
     }