Implement locale-agnostic number parsing (#1496)

This commit adds support for parsing numbers without being influenced by
the current system locale. We implement a from_chars
shim that forwards the call to strto*_l along with a statically
initialized locale constant.

Fixes #297
Fixes #379
Fixes #1322

Co-Authored-By: Patrick Hodoul <Patrick.Hodoul@autodesk.com>

Signed-off-by: L. E. Segovia <13498015+amyspark@users.noreply.github.com>

share/cmake/utils/CompilerFlags.cmake

@@ -25,6 +25,10 @@ if(USE_MSVC)
     # Note: Do not use /Wall (i.e. /W4) which adds 'informational' warnings.
     set(PLATFORM_COMPILE_FLAGS "${PLATFORM_COMPILE_FLAGS} /W3")
 
+    # Do enable C4701 (Potentially uninitialized local variable 'name' used), which is level 4.
+    # This is because strtoX-based from_chars leave the value variable unmodified.
+    set(PLATFORM_COMPILE_FLAGS "${PLATFORM_COMPILE_FLAGS} /we4701")
+
     if(OCIO_WARNING_AS_ERROR)
         set(PLATFORM_COMPILE_FLAGS "${PLATFORM_COMPILE_FLAGS} /WX")
     endif()

src/OpenColorIO/CMakeLists.txt

@@ -226,6 +226,7 @@ target_link_libraries(OpenColorIO
         ${OCIO_HALF_LIB}
         pystring::pystring
         sampleicc::sampleicc
+        utils::from_chars
         utils::strings
         yaml-cpp
 )

src/OpenColorIO/ParseUtils.cpp

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: BSD-3-Clause
 // Copyright Contributors to the OpenColorIO Project.
 
+#include <cmath>
 #include <cstring>
 #include <iostream>
 #include <set>
@@ -11,6 +12,7 @@
 #include "ParseUtils.h"
 #include "Platform.h"
 #include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 
 
 namespace OCIO_NAMESPACE
@@ -524,6 +526,7 @@ static constexpr int DOUBLE_DECIMALS = 16;
 std::string FloatToString(float value)
 {
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(FLOAT_DECIMALS);
     pretty << value;
     return pretty.str();
@@ -534,6 +537,7 @@ std::string FloatVecToString(const float * fval, unsigned int size)
     if(size<=0) return "";
 
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(FLOAT_DECIMALS);
     for(unsigned int i=0; i<size; ++i)
     {
@@ -548,9 +552,9 @@ bool StringToFloat(float * fval, const char * str)
 {
     if(!str) return false;
 
-    std::istringstream inputStringstream(str);
-    float x;
-    if(!(inputStringstream >> x))
+    float x = NAN;
+    const auto result = NumberUtils::from_chars(str, str + strlen(str), x);
+    if (result.ec != std::errc())
     {
         return false;
     }
@@ -565,6 +569,7 @@ bool StringToInt(int * ival, const char * str, bool failIfLeftoverChars)
     if(!ival) return false;
 
     std::istringstream i(str);
+    i.imbue(std::locale::classic());
     char c=0;
     if (!(i >> *ival) || (failIfLeftoverChars && i.get(c))) return false;
     return true;
@@ -573,6 +578,7 @@ bool StringToInt(int * ival, const char * str, bool failIfLeftoverChars)
 std::string DoubleToString(double value)
 {
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(DOUBLE_DECIMALS);
     pretty << value;
     return pretty.str();
@@ -583,6 +589,7 @@ std::string DoubleVecToString(const double * val, unsigned int size)
     if (size <= 0) return "";
 
     std::ostringstream pretty;
+    pretty.imbue(std::locale::classic());
     pretty.precision(DOUBLE_DECIMALS);
     for (unsigned int i = 0; i<size; ++i)
     {
@@ -599,9 +606,10 @@ bool StringVecToFloatVec(std::vector<float> &floatArray, const StringUtils::Stri
 
     for(unsigned int i=0; i<lineParts.size(); i++)
     {
-        std::istringstream inputStringstream(lineParts[i]);
-        float x;
-        if(!(inputStringstream >> x))
+        float x = NAN;
+        const char *str = lineParts[i].c_str();
+        const auto result = NumberUtils::from_chars(str, str + lineParts[i].size(), x);
+        if (result.ec != std::errc())
         {
             return false;
         }

src/OpenColorIO/fileformats/FileFormatHDL.cpp

@@ -37,6 +37,7 @@
 #include "ParseUtils.h"
 #include "transforms/FileTransform.h"
 #include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 
 
 namespace OCIO_NAMESPACE
@@ -182,16 +183,11 @@ readLuts(std::istream& istream,
         }
         else if(inlut)
         {
-            // StringToFloat was far slower, for 787456 values:
-            // - StringToFloat took 3879 (avg nanoseconds per value)
-            // - stdtod took 169 nanoseconds
-            char* endptr = 0;
-            float v = static_cast<float>(strtod(word.c_str(), &endptr));
+            float v{};
+            const auto result = NumberUtils::from_chars(word.c_str(), word.c_str() + word.size(), v);
 
-            if(!*endptr)
+            if (result.ec == std::errc())
             {
-                // Since each word should contain a single
-                // float value, the pointer should be null
                 lutValues[lutname].push_back(v);
             }
             else

src/OpenColorIO/fileformats/FileFormatIridasLook.cpp

@@ -16,6 +16,7 @@
 #include "pystring/pystring.h"
 #include "transforms/FileTransform.h"
 #include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 
 
 /*
@@ -416,19 +417,18 @@ class XMLParserHelper
             std::string size_raw = std::string(s, len);
             std::string size_clean = pystring::strip(size_raw, "'\" "); // strip quotes and space
 
-            char* endptr = 0;
-            int size_3d = static_cast<int>(strtol(size_clean.c_str(), &endptr, 10));
+            long int size_3d{};
+
+            const auto result = NumberUtils::from_chars(size_clean.c_str(), size_clean.c_str() + size_clean.size(), size_3d);
 
-            if (*endptr)
+            if (result.ec != std::errc())
             {
-                // strtol didn't consume entire string, there was
-                // remaining data, thus did not contain a single integer
                 std::ostringstream os;
                 os << "Invalid LUT size value: '" << size_raw;
                 os << "'. Expected quoted integer";
                 pImpl->Throw(os.str().c_str());
             }
-            pImpl->m_lutSize = size_3d;
+            pImpl->m_lutSize = static_cast<int>(size_3d);
         }
         else if (pImpl->m_data)
         {

src/OpenColorIO/fileformats/FileFormatSpi1D.cpp

@@ -1,7 +1,9 @@
 // SPDX-License-Identifier: BSD-3-Clause
 // Copyright Contributors to the OpenColorIO Project.
 
+#include <cmath>
 #include <cstdio>
+#include <cstring>
 #include <sstream>
 
 #include <OpenColorIO/OpenColorIO.h>
@@ -13,7 +15,7 @@
 #include "Platform.h"
 #include "transforms/FileTransform.h"
 #include "utils/StringUtils.h"
-
+#include "utils/NumberUtils.h"
 
 /*
 Version 1
@@ -124,10 +126,26 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
             }
             else if(StringUtils::StartsWith(headerLine, "From"))
             {
-                if (sscanf(lineBuffer, "From %f %f", &from_min, &from_max) != 2)
+                char fromMinS[64] = "";
+                char fromMaxS[64] = "";
+#ifdef _WIN32
+                if (sscanf_s(lineBuffer, "From %s %s", fromMinS, 64, fromMaxS, 64) != 2)
+#else
+                if (sscanf(lineBuffer, "From %s %s", fromMinS, fromMaxS) != 2)
+#endif
                 {
                     ThrowErrorMessage("Invalid 'From' Tag", currentLine, headerLine);
                 }
+                else
+                {
+                    const auto fromMinAnswer = NumberUtils::from_chars(fromMinS, fromMinS + 64, from_min);
+                    const auto fromMaxAnswer = NumberUtils::from_chars(fromMaxS, fromMaxS + 64, from_max);
+
+                    if (fromMinAnswer.ec != std::errc() || fromMaxAnswer.ec != std::errc())
+                    {
+                        ThrowErrorMessage("Invalid 'From' Tag", currentLine, headerLine);
+                    }
+                }
             }
             else if(StringUtils::StartsWith(headerLine, "Components"))
             {
@@ -179,7 +197,6 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
 
         int lineCount=0;
 
-        StringUtils::StringVec inputLUT;
         std::vector<float> values;
 
         while (istream.good())
@@ -192,10 +209,17 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
 
             if (line.length() != 0)
             {
-                inputLUT = StringUtils::SplitByWhiteSpaces(StringUtils::Trim(lineBuffer));
                 values.clear();
-                if (!StringVecToFloatVec(values, inputLUT)
-                    || components != (int)values.size())
+
+                char inputLUT[4][64] = {"", "", "", ""};
+#ifdef _WIN32
+                if (sscanf_s(lineBuffer, "%s %s %s %63s", inputLUT[0], 64,
+                           inputLUT[1], 64, inputLUT[2], 64, inputLUT[3],
+                           64) != components)
+#else
+                if (sscanf(lineBuffer, "%s %s %s %63s", inputLUT[0],
+                           inputLUT[1], inputLUT[2], inputLUT[3]) != components)
+#endif
                 {
                     std::ostringstream os;
                     os << "Malformed LUT line. Expecting a ";
@@ -209,6 +233,26 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
                     ThrowErrorMessage("Too many entries found", currentLine, "");
                 }
 
+                values.resize(components);
+
+                for (int i = 0; i < components; i++)
+                {
+                    float v = NAN;
+                    const auto result = NumberUtils::from_chars(inputLUT[i], inputLUT[i] + 64, v);
+
+                    if (result.ec != std::errc())
+                    {
+                        std::ostringstream os;
+                        os << "Malformed LUT line. Could not convert component";
+                        os << i << " to a floating point number.";
+
+                        ThrowErrorMessage("Malformed LUT line", currentLine,
+                                            line);
+                    }
+
+                    values[i] = v;
+                }
+
                 // If 1 component is specified, use x1 x1 x1.
                 if (components == 1)
                 {

src/OpenColorIO/fileformats/FileFormatSpi3D.cpp

@@ -12,6 +12,7 @@
 #include "Platform.h"
 #include "transforms/FileTransform.h"
 #include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 
 
 /*
@@ -131,7 +132,7 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
     // Parse table
     int index = 0;
     int rIndex, gIndex, bIndex;
-    float redValue, greenValue, blueValue;
+    float redValue = NAN, greenValue = NAN, blueValue = NAN;
 
     int entriesRemaining = rSize * gSize * bSize;
     Array & lutArray = lut3d->getArray();
@@ -141,10 +142,42 @@ CachedFileRcPtr LocalFileFormat::read(std::istream & istream,
     {
         istream.getline(lineBuffer, MAX_LINE_SIZE);
 
-        if (sscanf(lineBuffer, "%d %d %d %f %f %f",
+        char redValueS[64] = "";
+        char greenValueS[64] = "";
+        char blueValueS[64] = "";
+
+#ifdef _WIN32
+        if (sscanf(lineBuffer,
+            "%d %d %d %s %s %s",
+            &rIndex, &gIndex, &bIndex,
+            redValueS, 64,
+            greenValueS, 64,
+            blueValueS, 64) == 6)
+#else
+        if (sscanf(lineBuffer, "%d %d %d %s %s %s",
             &rIndex, &gIndex, &bIndex,
-            &redValue, &greenValue, &blueValue) == 6)
+            redValueS, greenValueS, blueValueS) == 6)
+#endif
         {
+            const auto redValueAnswer = NumberUtils::from_chars(redValueS, redValueS + 64, redValue);
+            const auto greenValueAnswer = NumberUtils::from_chars(greenValueS, greenValueS + 64, greenValue);
+            const auto blueValueAnswer = NumberUtils::from_chars(blueValueS, blueValueS + 64, blueValue);
+
+            if (redValueAnswer.ec != std::errc()
+                || greenValueAnswer.ec != std::errc()
+                || blueValueAnswer.ec != std::errc())
+            {
+                std::ostringstream os;
+                os << "Error parsing .spi3d file (";
+                os << fileName;
+                os << "). ";
+                os << "Data is invalid. ";
+                os << "A color value is specified (";
+                os << redValueS << " " << greenValueS << " " << blueValueS;
+                os << ") that cannot be parsed as a floating-point triplet.";
+                throw Exception(os.str().c_str());
+            }
+
             bool invalidIndex = false;
             if (rIndex < 0 || rIndex >= rSize
                 || gIndex < 0 || gIndex >= gSize

src/OpenColorIO/fileformats/xmlutils/XMLReaderUtils.h

@@ -13,6 +13,8 @@
 #include <OpenColorIO/OpenColorIO.h>
 
 #include "MathUtils.h"
+#include "utils/StringUtils.h"
+#include "utils/NumberUtils.h"
 #include "Platform.h"
 
 
@@ -156,16 +158,17 @@ void ParseNumber(const char * str, size_t startPos, size_t endPos, T & value)
     const char * startParse = str + startPos;
 
     double val = 0.0f;
-    char * endParse = nullptr;
-
-    // The strtod expects a C string and str might not be null terminated.
-    // However since strtod will stop parsing when it encounters characters
-    // that it cannot convert to a number, in practice it does not need to
-    // be null terminated.
-    // C++11 version of strtod processes NAN & INF ASCII values.
-    val = strtod(startParse, &endParse);
+
+    size_t adjustedStartPos = startPos;
+    size_t adjustedEndPos = endPos;
+
+    FindSubString(startParse, endPos - startPos, adjustedStartPos, adjustedEndPos);
+
+    const auto result = NumberUtils::from_chars(startParse + adjustedStartPos, startParse + adjustedEndPos, val);
+
     value = (T)val;
-    if (endParse == startParse)
+
+    if (result.ec == std::errc::invalid_argument)
     {
         std::string fullStr(str, endPos);
         std::string parsedStr(startParse, endPos - startPos);
@@ -187,10 +190,10 @@ void ParseNumber(const char * str, size_t startPos, size_t endPos, T & value)
             << TruncateString(fullStr.c_str(), endPos, 100) << "'.";
         throw Exception(oss.str().c_str());
     }
-    else if (endParse != str + endPos)
+    else if (result.ptr != str + endPos)
     {
         // Number is followed by something.
-        std::string fullStr(str, startPos + (endParse - startParse));
+        std::string fullStr(str, endPos);
         std::string parsedStr(startParse, endPos - startPos);
         std::ostringstream oss;
         oss << "ParserNumber: '"

src/utils/CMakeLists.txt

@@ -15,3 +15,12 @@ configure_file("Half.h.in" "Half.h" @ONLY)
 set_property(TARGET ${OCIO_HALF_LIB} APPEND PROPERTY
     INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_CURRENT_BINARY_DIR}/.."
 )
+
+# from_chars shim (via strtod_l)
+
+add_library(utils::from_chars INTERFACE IMPORTED GLOBAL)
+
+set_target_properties(utils::from_chars PROPERTIES
+    INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_CURRENT_SOURCE_DIR}/.."
+)
+