More principled handling of string<->numeric conversions.

.travis.yml

@@ -32,6 +32,7 @@ addons:
       - libavformat-dev
       - libswscale-dev
       - libavutil-dev
+      - locales
       # - qtbase5-dev   # FIXME: enable Qt5 on Linux
       # - bzip2
       # - libtinyxml-dev

src/fits.imageio/fitsinput.cpp

@@ -348,7 +348,7 @@ FitsInput::add_to_spec (const std::string &keyname, const std::string &value)
     // converting string to float or integer
     bool isNumSign = (value[0] == '+' || value[0] == '-' || value[0] == '.');
     if (isdigit (value[0]) || isNumSign) {
-        float val = atof (value.c_str ());
+        float val = Strutil::stof (value);
         if (val == (int)val)
             m_spec.attribute (keyname, (int)val);
         else

src/include/OpenImageIO/optparser.h

@@ -40,6 +40,7 @@
 #define OPENIMAGEIO_OPTPARSER_H
 
 #include <string>
+#include <OpenImageIO/strutil.h>
 
 OIIO_NAMESPACE_BEGIN
 
@@ -67,9 +68,9 @@ optparse1 (C &system, const std::string &opt)
     char v = value.size() ? value[0] : ' ';
     if ((v >= '0' && v <= '9') || v == '+' || v == '-') {  // numeric
         if (strchr (value.c_str(), '.'))  // float
-            return system.attribute (name.c_str(), (float)atof(value.c_str()));
+            return system.attribute (name, Strutil::stof(value));
         else  // int
-            return system.attribute (name.c_str(), (int)atoi(value.c_str()));
+            return system.attribute (name, Strutil::stoi(value));
     }
     // otherwise treat it as a string
 

src/include/OpenImageIO/strutil.h

@@ -97,6 +97,8 @@ void OIIO_API sync_output (std::ostream &file, string_view str);
 ///
 /// Uses the tinyformat library underneath, so it's fully type-safe, and
 /// works with any types that understand stream output via '<<'.
+/// The formatting of the string will always use the classic "C" locale
+/// conventions (in particular, '.' as decimal separator for float values).
 template<typename... Args>
 inline std::string format (string_view fmt, const Args&... args)
 {
@@ -105,23 +107,25 @@ inline std::string format (string_view fmt, const Args&... args)
 
 
 /// Output formatted string to stdout, type-safe, and threads can't clobber
-/// one another.
+/// one another. This will force the classic "C" locale.
 template<typename... Args>
 inline void printf (string_view fmt, const Args&... args)
 {
     sync_output (stdout, format(fmt, args...));
 }
 
+
 /// Output formatted string to an open FILE*, type-safe, and threads can't
-/// clobber one another.
+/// clobber one another.  This will force classic "C" locale conventions.
 template<typename... Args>
 inline void fprintf (FILE *file, string_view fmt, const Args&... args)
 {
     sync_output (file, format(fmt, args...));
 }
 
+
 /// Output formatted string to an open ostream, type-safe, and threads can't
-/// clobber one another.
+/// clobber one another. This will force classic "C" locale conventions.
 template<typename... Args>
 inline void fprintf (std::ostream &file, string_view fmt, const Args&... args)
 {
@@ -254,50 +258,99 @@ std::string OIIO_API repeat (string_view str, int n);
 std::string OIIO_API replace (string_view str, string_view pattern,
                               string_view replacement, bool global=false);
 
-// Helper template to test if a string is a generic type
-template<typename T>
-inline bool string_is (string_view /*s*/) {
-    return false; // Generic: assume there is an explicit specialization
-}
-// Special case for int
-template <> inline bool string_is<int> (string_view s) {
-    if (s.empty())
-        return false;
-    char *endptr = 0;
-    strtol (s.data(), &endptr, 10);
-    return (s.data() + s.size() == endptr);
-}
-// Special case for float
-template <> inline bool string_is<float> (string_view s) {
-    if (s.empty())
-        return false;
-    char *endptr = 0;
-    strtod (s.data(), &endptr);
-    return (s.data() + s.size() == endptr);
+
+/// strtod/strtof equivalents that are "locale-independent", always using
+/// '.' as the decimal separator. This should be preferred for I/O and other
+/// situations where you want the same standard formatting regardless of
+/// locale.
+float OIIO_API strtof (const char *nptr, char **endptr = nullptr);
+double OIIO_API strtod (const char *nptr, char **endptr = nullptr);
+
+
+// stoi() returns the int conversion of text from a string.
+// No exceptions or errors -- parsing errors just return 0, over/underflow
+// gets clamped to int range. No locale consideration.
+OIIO_API int stoi (string_view s, size_t* pos=0, int base=10);
+
+// stoui() returns the unsigned int conversion of text from a string.
+// No exceptions or errors -- parsing errors just return 0. Negative
+// values are cast, overflow is clamped. No locale considerations.
+inline unsigned int stoui (string_view s, size_t* pos=0, int base=10) {
+    return static_cast<unsigned int>(stoi (s, pos, base));
 }
 
+/// stof() returns the float conversion of text from several string types.
+/// No exceptions or errors -- parsing errors just return 0.0. These always
+/// use '.' for the decimal mark (versus atof and std::strtof, which are
+/// locale-dependent).
+OIIO_API float stof (string_view s, size_t* pos=0);
+#define OIIO_STRUTIL_HAS_STOF 1  /* be able to test this */
+
+// Temporary fix: allow separate std::string and char* versions, to avoid
+// string_view allocation on some platforms. This will be deprecated once
+// we can count on all supported compilers using short string optimization.
+OIIO_API float stof (const std::string& s, size_t* pos=0);
+OIIO_API float stof (const char* s, size_t* pos=0);
+// N.B. For users of ustring, there's a stof(ustring) defined in ustring.h.
+
+OIIO_API double stod (string_view s, size_t* pos=0);
+OIIO_API double stod (const std::string& s, size_t* pos=0);
+OIIO_API double stod (const char* s, size_t* pos=0);
+
 
 
-// Helper template to convert from generic type to string
+/// Return true if the string is exactly (other than leading  and trailing
+/// whitespace) a valid int.
+OIIO_API bool string_is_int (string_view s);
+
+/// Return true if the string is exactly (other than leading or trailing
+/// whitespace) a valid float. This operations in a locale-independent
+/// manner, i.e., it assumes '.' as the decimal mark.
+OIIO_API bool string_is_float (string_view s);
+
+
+
+// Helper template to convert from generic type to string. Used when you
+// want stoX but you're in a template. Rigged to use "C" locale.
 template<typename T>
 inline T from_string (string_view s) {
     return T(s); // Generic: assume there is an explicit converter
 }
 // Special case for int
 template<> inline int from_string<int> (string_view s) {
-    return s.size() ? strtol (s.c_str(), NULL, 10) : 0;
+    return Strutil::stoi(s);
 }
 // Special case for uint
 template<> inline unsigned int from_string<unsigned int> (string_view s) {
-    return s.size() ? strtoul (s.c_str(), NULL, 10) : (unsigned int)0;
+    return Strutil::stoui(s);
 }
-// Special case for float
+// Special case for float -- note that by using Strutil::strtof, this
+// always treats '.' as the decimal mark.
 template<> inline float from_string<float> (string_view s) {
-    return s.size() ? (float)strtod (s.c_str(), NULL) : 0.0f;
+    return Strutil::stof(s);
 }
 
 
 
+// Helper template to test if a string is a generic type. Used instead of
+// string_is_X, but when you're inside templated code.
+template<typename T>
+inline bool string_is (string_view /*s*/) {
+    return false; // Generic: assume there is an explicit specialization
+}
+// Special case for int
+template <> inline bool string_is<int> (string_view s) {
+    return string_is_int (s);
+}
+// Special case for float. Note that by using Strutil::stof, this always
+// treats '.' as the decimal character.
+template <> inline bool string_is<float> (string_view s) {
+    return string_is_float (s);
+}
+
+
+
+
 /// Given a string containing values separated by a comma (or optionally
 /// another separator), extract the individual values, placing them into
 /// vals[] which is presumed to already contain defaults.  If only a single

src/include/OpenImageIO/tinyformat.h

@@ -266,6 +266,7 @@ template<typename T>
 inline void formatTruncated(std::ostream& out, const T& value, int ntrunc)
 {
     std::ostringstream tmp;
+    tmp.imbue (out.getloc());
     tmp << value;
     std::string result = tmp.str();
     out.write(result.c_str(), (std::min)(ntrunc, static_cast<int>(result.size())));
@@ -800,6 +801,7 @@ inline void formatImpl(std::ostream& out, const char* fmt,
             // it crudely by formatting into a temporary string stream and
             // munging the resulting string.
             std::ostringstream tmpStream;
+            tmpStream.imbue (out.getloc());
             tmpStream.copyfmt(out);
             tmpStream.setf(std::ios::showpos);
             arg.format(tmpStream, fmt, fmtEnd, ntrunc);
@@ -931,6 +933,7 @@ TINYFORMAT_FOREACH_ARGNUM(TINYFORMAT_MAKE_MAKEFORMATLIST)
 #endif
 
 /// Format list of arguments to the stream according to the given format string.
+/// This honors the stream's existing locale conventions.
 ///
 /// The name vformat() is chosen for the semantic similarity to vprintf(): the
 /// list of format arguments is held in a single function argument.
@@ -943,23 +946,40 @@ inline void vformat(std::ostream& out, const char* fmt, FormatListRef list)
 #ifdef TINYFORMAT_USE_VARIADIC_TEMPLATES
 
 /// Format list of arguments to the stream according to given format string.
+/// This honors the stream's existing locale conventions.
 template<typename... Args>
 void format(std::ostream& out, const char* fmt, const Args&... args)
 {
     vformat(out, fmt, makeFormatList(args...));
 }
 
 /// Format list of arguments according to the given format string and return
-/// the result as a string.
+/// the result as a string, using classic "C" locale conventions (e.g.,
+/// using '.' as decimal mark).
 template<typename... Args>
 std::string format(const char* fmt, const Args&... args)
 {
     std::ostringstream oss;
+    oss.imbue (std::locale::classic());  // force "C" locale with '.' decimal
+    format(oss, fmt, args...);
+    return oss.str();
+}
+
+/// Format list of arguments according to the given format string and return
+/// the result as a string, using an explicit locale. Passing loc as a
+/// default-constructed std::locale will result in adhering to the current
+/// "native" locale set by the OS.
+template<typename... Args>
+std::string format(const std::locale& loc, const char* fmt, const Args&... args)
+{
+    std::ostringstream oss;
+    oss.imbue (loc);
     format(oss, fmt, args...);
     return oss.str();
 }
 
 /// Format list of arguments to std::cout, according to the given format string
+/// This honors std::out's existing locale conventions.
 template<typename... Args>
 void printf(const char* fmt, const Args&... args)
 {
@@ -1011,6 +1031,16 @@ template<TINYFORMAT_ARGTYPES(n)>                                          \
 std::string format(const char* fmt, TINYFORMAT_VARARGS(n))                \
 {                                                                         \
     std::ostringstream oss;                                               \
+    oss.imbue (std::locale::classic());                                   \
+    format(oss, fmt, TINYFORMAT_PASSARGS(n));                             \
+    return oss.str();                                                     \
+}                                                                         \
+                                                                          \
+template<TINYFORMAT_ARGTYPES(n)>                                          \
+std::string format(const std::locale& loc, const char* fmt, TINYFORMAT_VARARGS(n)) \
+{                                                                         \
+    std::ostringstream oss;                                               \
+    oss.imbue (loc);                                                      \
     format(oss, fmt, TINYFORMAT_PASSARGS(n));                             \
     return oss.str();                                                     \
 }                                                                         \

src/include/OpenImageIO/ustring.h

@@ -626,6 +626,8 @@ class OIIO_API ustring {
     /// something like:
     ///    ustring s = ustring::format ("blah %d %g", (int)foo, (float)bar);
     /// The argument list is fully typesafe.
+    /// The formatting of the string will always use the classic "C" locale
+    /// conventions (in particular, '.' as decimal separator for float values).
     template<typename... Args>
     static ustring format (string_view fmt, const Args&... args)
     {
@@ -751,6 +753,12 @@ inline bool iequals (const std::string &a, ustring b) {
 }
 
 
+
+// ustring variant stof from OpenImageIO/strutil.h
+namespace Strutil {
+inline int stof (ustring s) { return Strutil::stof (s.string()); }
+} // end namespace Strutil
+
 OIIO_NAMESPACE_END
 
 #endif // OPENIMAGEIO_USTRING_H

src/iv/imageviewer.cpp

@@ -117,12 +117,9 @@ ImageViewer::ImageViewer ()
 {
     readSettings (false);
 
-    const char *gamenv = getenv ("GAMMA");
-    if (gamenv) {
-        float g = atof (gamenv);
-        if (g >= 0.1 && g <= 5)
-            m_default_gamma = g;
-    }
+    float gam = Strutil::stof (Sysutil::getenv ("GAMMA"));
+    if (gam >= 0.1 && gam <= 5)
+        m_default_gamma = gam;
     // FIXME -- would be nice to have a more nuanced approach to display
     // color space, in particular knowing whether the display is sRGB.
     // Also, some time in the future we may want a real 3D LUT for 

src/libOpenImageIO/formatspec.cpp

@@ -936,6 +936,7 @@ spec_to_xml (const ImageSpec &spec, ImageSpec::SerialVerbose verbose)
     }
 
     std::ostringstream result;
+    result.imbue (std::locale::classic());  // force "C" locale with '.' decimal
     doc.print (result, "");
     return result.str();
 }

src/libOpenImageIO/maketexture.cpp

@@ -1386,6 +1386,7 @@ make_texture_impl (ImageBufAlgo::MakeTextureMode mode,
     // (such as filtering information) needs to be manually added into the
     // hash.
     std::ostringstream addlHashData;
+    addlHashData.imbue (std::locale::classic()); // Force "C" locale with '.' decimal
     addlHashData << filtername << " ";
     float sharpen = configspec.get_float_attribute ("maketx:sharpen", 0.0f);
     if (sharpen != 0.0f) {
@@ -1417,6 +1418,7 @@ make_texture_impl (ImageBufAlgo::MakeTextureMode mode,
 
     if (isConstantColor) {
         std::ostringstream os; // Emulate a JSON array
+        os.imbue (std::locale::classic());  // Force "C" locale with '.' decimal
         for (int i = 0; i < dstspec.nchannels; ++i) {
             if (i!=0) os << ",";
             os << (i<(int)constantColor.size() ? constantColor[i] : 0.0f);
@@ -1436,6 +1438,7 @@ make_texture_impl (ImageBufAlgo::MakeTextureMode mode,
 
     if (compute_average_color) {
         std::ostringstream os; // Emulate a JSON array
+        os.imbue (std::locale::classic());  // Force "C" locale with '.' decimal
         for (int i = 0; i < dstspec.nchannels; ++i) {
             if (i!=0) os << ",";
             os << (i<(int)pixel_stats.avg.size() ? pixel_stats.avg[i] : 0.0f);

src/libtexture/imagecache.cpp

@@ -1610,6 +1610,7 @@ ImageCacheImpl::onefile_stat_line (const ImageCacheFileRef &file,
 {
     // FIXME -- make meaningful stat printouts for multi-image textures
     std::ostringstream out;
+    out.imbue (std::locale::classic());  // Force "C" locale with '.' decimal
     const ImageSpec &spec (file->spec(0,0));
     const char *formatcode = "u8";
     switch (spec.format.basetype) {
@@ -1737,6 +1738,7 @@ ImageCacheImpl::getstats (int level) const
     }
 
     std::ostringstream out;
+    out.imbue (std::locale::classic());  // Force "C" locale with '.' decimal
     if (level > 0) {
         out << "OpenImageIO ImageCache statistics (";
         {

src/libtexture/texturesys.cpp

@@ -379,6 +379,7 @@ TextureSystemImpl::getstats (int level, bool icstats) const
     m_imagecache->mergestats (stats);
 
     std::ostringstream out;
+    out.imbue (std::locale::classic());  // Force "C" locale with '.' decimal
     bool anytexture = (stats.texture_queries + stats.texture3d_queries +
                        stats.shadow_queries + stats.environment_queries);
     if (level > 0 && anytexture) {

src/libutil/argparse.cpp

@@ -252,11 +252,11 @@ ArgOption::set_parameter (int i, const char *argv)
 
     case 'f':
     case 'g':
-        *(float *)m_param[i] = (float)atof(argv);
+        *(float *)m_param[i] = Strutil::stof(argv);
         break;
 
     case 'F':
-        *(double *)m_param[i] = atof(argv);
+        *(double *)m_param[i] = Strutil::stod(argv);
         break;
 
     case 's':