Fix from_chars in C++17 code path to understand hex prefix (0x) and s…

…kip optional whitespace To match the pre-C++17 behavior that was there before Signed-off-by: Aras Pranckevicius <[email protected]>
AcademySoftwareFoundation · Oct 2, 2024 · 8428381 · 8428381
1 parent 8f5be6a
commit 8428381
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 23 deletions.
diff --git a/src/utils/NumberUtils.h b/src/utils/NumberUtils.h
@@ -11,7 +11,7 @@
 //
 // Except on Apple platforms, where (as of Xcode 15 / Apple Clang 15)
 // these are not implemented for float/double types.
-#if __cpp_lib_to_chars >= 201611L && !defined(__APPLE__)
+#if __cplusplus >= 201703L && !defined(__APPLE__)
 #define USE_CHARCONV_FROM_CHARS
 #include <charconv>
 #endif
@@ -66,6 +66,37 @@ struct from_chars_result
 
 static const Locale loc;
 
+#ifdef USE_CHARCONV_FROM_CHARS
+really_inline bool from_chars_is_space(char c) noexcept
+{
+    return c == ' ' || c == '\n' || c == '\t' || c == '\r' || c == '\v' || c == '\f';
+}
+
+// skip prefix whitespace and "+"
+really_inline const char* from_chars_skip_prefix(const char* first, const char* last) noexcept
+{
+    while (first < last && from_chars_is_space(first[0]))
+    {
+        ++first;
+    }
+    if (first < last && first[0] == '+')
+    {
+        ++first;
+    }
+    return first;
+}
+
+really_inline bool from_chars_hex_prefix(const char*& first, const char* last) noexcept
+{
+    if (first + 2 < last && first[0] == '0' && (first[1] == 'x' || first[1] == 'X'))
+    {
+        first += 2;
+        return true;
+    }
+    return false;
+}
+#endif
+
 really_inline from_chars_result from_chars(const char *first, const char *last, double &value) noexcept
 {
     if (!first || !last || first == last)
@@ -74,11 +105,13 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     }
 
 #ifdef USE_CHARCONV_FROM_CHARS
-    if (first < last && first[0] == '+')
+    first = from_chars_skip_prefix(first, last);
+    std::chars_format fmt = std::chars_format::general;
+    if (from_chars_hex_prefix(first, last))
     {
-        ++first;
+        fmt = std::chars_format::hex;
     }
-    std::from_chars_result res = std::from_chars(first, last, value);
+    std::from_chars_result res = std::from_chars(first, last, value, fmt);
     return from_chars_result{ res.ptr, res.ec };
 #else
 
@@ -120,11 +153,13 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     }
 
 #ifdef USE_CHARCONV_FROM_CHARS
-    if (first < last && first[0] == '+')
+    first = from_chars_skip_prefix(first, last);
+    std::chars_format fmt = std::chars_format::general;
+    if (from_chars_hex_prefix(first, last))
     {
-        ++first;
+        fmt = std::chars_format::hex;
     }
-    std::from_chars_result res = std::from_chars(first, last, value);
+    std::from_chars_result res = std::from_chars(first, last, value, fmt);
     return from_chars_result{ res.ptr, res.ec };
 #else
 
@@ -174,11 +209,13 @@ really_inline from_chars_result from_chars(const char *first, const char *last,
     }
 
 #ifdef USE_CHARCONV_FROM_CHARS
-    if (first < last && first[0] == '+')
+    first = from_chars_skip_prefix(first, last);
+    int base = 10;
+    if (from_chars_hex_prefix(first, last))
     {
-        ++first;
+        base = 16;
     }
-    std::from_chars_result res = std::from_chars(first, last, value);
+    std::from_chars_result res = std::from_chars(first, last, value, base);
     return from_chars_result{ res.ptr, res.ec };
 #else
 

diff --git a/tests/cpu/fileformats/xmlutils/XMLReaderUtils_tests.cpp b/tests/cpu/fileformats/xmlutils/XMLReaderUtils_tests.cpp
@@ -46,12 +46,21 @@ OCIO_ADD_TEST(XMLReaderHelper, string_to_float_failure)
     const char str2[] = "12345";
     const size_t len2 = std::strlen(str2);
     // All characters are parsed and this is more than the required length.
-    // The string to double function strtod does not stop at a given length,
-    // but we detect that strtod did read too many characters.
-    OCIO_CHECK_THROW_WHAT(OCIO::ParseNumber(str2, 0, len2 - 2, value),
-                          OCIO::Exception,
-                          "followed by unexpected characters");
-
+    // The string to double function might not stop at a given length,
+    // but we detect if it did read too many characters.
+    try
+    {
+        OCIO::ParseNumber(str2, 0, len2 - 2, value);
+        // At this point value should be 123 if underlying implementation
+        // properly stops at given length.
+        OCIO_CHECK_EQUAL(value, 123.0f);
+    }
+    catch (OCIO::Exception const& ex)
+    {
+        // If underlying implementation scans past the end, exception should be thrown.
+        const std::string what(ex.what());
+        OCIO_CHECK_ASSERT(what.find("followed by unexpected characters") != std::string::npos);
+    }
 
     const char str3[] = "123XX";
     const size_t len3 = std::strlen(str3);
@@ -385,13 +394,6 @@ OCIO_ADD_TEST(XMLReaderHelper, parse_number)
         OCIO_CHECK_EQUAL(data, 1.0f);
     }
 
-    {
-        std::string buffer(" 123 ");
-        OCIO_CHECK_THROW_WHAT(OCIO::ParseNumber(buffer.c_str(),
-                                                0, 3, data),
-                              OCIO::Exception,
-                              "followed by unexpected characters");
-    }
     {
         std::string buffer("XY");
         OCIO_CHECK_THROW_WHAT(OCIO::ParseNumber(buffer.c_str(),