From d9ab3192cc9f5a8d1141a26946837977a9f182a0 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Tue, 10 Oct 2017 14:25:49 -0700 Subject: [PATCH] More principled handling of string<->numeric conversions. This refactor comes after a belated realization that our numeric conversion routines (principally from_string<> and format) are "locale-dependent" via their use of strtod, as well as atof() scattered through the code. In particular, this means that when software using this code is running with a locale in which the decimal mark is something other than '.' (e.g., many European locales that use ',' for decimal mark), these functions will incorrectly parse text floating point numbers that use the standard period as decimal mark. For example, if the locale is "fr_FR", atof("123.45") will return 123.0, rather than the correct 123.45. After much debate, and a few implementations, we are declaring that since the whole point of OIIO is about reading and writing persistent data (image files!), that in order to be consistent, ALL of OIIO's string parsing and formatting, including that done by utility library functions, will be locale-independent by using the "C" locale (in particular, we will use the usual North American default of '.' for decimal separator). Additionally: * Added a new stof(), stoi(), stoul() to replace the clunkier template syntax of from_string<>. As explained, these are hard-coded to "C" locale, not the native/global locale. * Much more attention to string_view safety, finding a number of subtle bugs (including Strutil::parse_int, etc.) where it was definitely not safe to assume that the string_view passed was the null-terminated edge of the string, and so, for example if a string_view pointed to the characters "12" but the very next characters were "345" parse_int would return 12345 instead of the correct 12. * Strutil::format and ustring::format (and our copy of tinyformat underneath) were touched up so that the versions of format() that return strings will use the "C" locale, versions added that return strings and use an explicitly-passed locale, and the versions that append their results to existing streams continue to operate as before by honoring the existing locale conventions of the streams they are using. We may revise this later depending on what the upstream tinyformat chooses to do about the issue. * Several places where we assembled strings using std::ostringstream, I forced the stream to use classic "C" locale in cases where it was likely to have any floating-point data output. * For maketx and oiiotool, globally set the locale to classic. In these cases, we control the whole app, so this is safe to do. If we ever decide that we want versions of these functions that take explicit locales, it's not hard to add. --- .travis.yml | 1 + src/fits.imageio/fitsinput.cpp | 2 +- src/include/OpenImageIO/optparser.h | 5 +- src/include/OpenImageIO/strutil.h | 109 +++++++--- src/include/OpenImageIO/tinyformat.h | 32 ++- src/include/OpenImageIO/ustring.h | 8 + src/iv/imageviewer.cpp | 9 +- src/libOpenImageIO/formatspec.cpp | 1 + src/libOpenImageIO/maketexture.cpp | 3 + src/libtexture/imagecache.cpp | 2 + src/libtexture/texturesys.cpp | 1 + src/libutil/argparse.cpp | 4 +- src/libutil/benchmark.cpp | 16 +- src/libutil/filesystem.cpp | 10 +- src/libutil/strutil.cpp | 312 +++++++++++++++++++++++++-- src/libutil/strutil_test.cpp | 173 ++++++++++++--- src/libutil/ustring.cpp | 1 + src/maketx/maketx.cpp | 4 + src/oiiotool/oiiotool.cpp | 92 ++++---- src/rla.imageio/rlainput.cpp | 2 +- 20 files changed, 631 insertions(+), 156 deletions(-) diff --git a/.travis.yml b/.travis.yml index 43d27228ad..7996c868cf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,6 +32,7 @@ addons: - libavformat-dev - libswscale-dev - libavutil-dev + - locales # - qtbase5-dev # FIXME: enable Qt5 on Linux # - bzip2 # - libtinyxml-dev diff --git a/src/fits.imageio/fitsinput.cpp b/src/fits.imageio/fitsinput.cpp index a8e1218b96..80f811b22e 100644 --- a/src/fits.imageio/fitsinput.cpp +++ b/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 diff --git a/src/include/OpenImageIO/optparser.h b/src/include/OpenImageIO/optparser.h index dcf8ce0349..6c57ec786e 100644 --- a/src/include/OpenImageIO/optparser.h +++ b/src/include/OpenImageIO/optparser.h @@ -40,6 +40,7 @@ #define OPENIMAGEIO_OPTPARSER_H #include +#include 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 diff --git a/src/include/OpenImageIO/strutil.h b/src/include/OpenImageIO/strutil.h index 6d034d5067..901cd056c8 100644 --- a/src/include/OpenImageIO/strutil.h +++ b/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 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 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 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 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 -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 (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 (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(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 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 (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 (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 (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 +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 (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 (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 diff --git a/src/include/OpenImageIO/tinyformat.h b/src/include/OpenImageIO/tinyformat.h index 0449875885..f6cc94680b 100644 --- a/src/include/OpenImageIO/tinyformat.h +++ b/src/include/OpenImageIO/tinyformat.h @@ -266,6 +266,7 @@ template 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(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,6 +946,7 @@ 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 void format(std::ostream& out, const char* fmt, const Args&... args) { @@ -950,16 +954,32 @@ void format(std::ostream& out, const char* fmt, const Args&... 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 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 +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 void printf(const char* fmt, const Args&... args) { @@ -1011,6 +1031,16 @@ template \ 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 \ +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(); \ } \ diff --git a/src/include/OpenImageIO/ustring.h b/src/include/OpenImageIO/ustring.h index 59b0dd26a8..ca2bdb7dac 100644 --- a/src/include/OpenImageIO/ustring.h +++ b/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 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 diff --git a/src/iv/imageviewer.cpp b/src/iv/imageviewer.cpp index 91c898eae4..f0074d8c59 100644 --- a/src/iv/imageviewer.cpp +++ b/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 diff --git a/src/libOpenImageIO/formatspec.cpp b/src/libOpenImageIO/formatspec.cpp index 4afb42c4bc..1d0a9debcc 100644 --- a/src/libOpenImageIO/formatspec.cpp +++ b/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(); } diff --git a/src/libOpenImageIO/maketexture.cpp b/src/libOpenImageIO/maketexture.cpp index b566291870..3e87af68a7 100644 --- a/src/libOpenImageIO/maketexture.cpp +++ b/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); diff --git a/src/libtexture/imagecache.cpp b/src/libtexture/imagecache.cpp index 17330f5771..60fec4fd8f 100644 --- a/src/libtexture/imagecache.cpp +++ b/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 ("; { diff --git a/src/libtexture/texturesys.cpp b/src/libtexture/texturesys.cpp index c3275f375f..17c4936f19 100644 --- a/src/libtexture/texturesys.cpp +++ b/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) { diff --git a/src/libutil/argparse.cpp b/src/libutil/argparse.cpp index b128b728b6..77d131da28 100644 --- a/src/libutil/argparse.cpp +++ b/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': diff --git a/src/libutil/benchmark.cpp b/src/libutil/benchmark.cpp index f4259457fa..6099a0860c 100644 --- a/src/libutil/benchmark.cpp +++ b/src/libutil/benchmark.cpp @@ -144,6 +144,12 @@ std::ostream& operator<< (std::ostream& out, const Benchmarker &bench) } const char* unitname = unitnames[unit]; double scale = unitscales[unit]; + char rateunit = 'M'; + double ratescale = 1.0e6; + if (bench.avg() >= 1.0e-6) { + rateunit = 'k'; + ratescale = 1.0e3; + } avg *= scale; stddev *= scale; @@ -163,12 +169,12 @@ std::ostream& operator<< (std::ostream& out, const Benchmarker &bench) return out; } if (bench.work() == 1) - out << Strutil::format ("%6.1f M/s", - (1.0f/1.0e6)/bench.avg()); + out << Strutil::format ("%6.1f %c/s", + (1.0f/ratescale)/bench.avg(), rateunit); else - out << Strutil::format ("%6.1f Mvals/s, %.1f Mcalls/s", - (bench.work()/1.0e6)/bench.avg(), - (1.0f/1.0e6)/bench.avg()); + out << Strutil::format ("%6.1f %cvals/s, %.1f %ccalls/s", + (bench.work()/ratescale)/bench.avg(), rateunit, + (1.0f/ratescale)/bench.avg(), rateunit); if (bench.verbose() >= 2) out << Strutil::format (" (%dx%d, rng=%.1f%%, med=%.1f)", bench.trials(), bench.iterations(), unitname, diff --git a/src/libutil/filesystem.cpp b/src/libutil/filesystem.cpp index 8db1142664..cb591a3001 100644 --- a/src/libutil/filesystem.cpp +++ b/src/libutil/filesystem.cpp @@ -723,16 +723,16 @@ Filesystem::enumerate_sequence (string_view desc, std::vector &numbers) // If 'y' is used, generate the complement. std::vector range; Strutil::split (s, range, "-", 2); - int first = Strutil::from_string (range[0]); + int first = Strutil::stoi (range[0]); int last = first; int step = 1; bool complement = false; if (range.size() > 1) { - last = Strutil::from_string (range[1]); + last = Strutil::stoi (range[1]); if (const char *x = strchr (range[1].c_str(), 'x')) - step = (int) strtol (x+1, NULL, 10); + step = Strutil::stoi (x+1); else if (const char *x = strchr (range[1].c_str(), 'y')) { - step = (int) strtol (x+1, NULL, 10); + step = Strutil::stoi (x+1); complement = true; } if (step == 0) @@ -989,7 +989,7 @@ Filesystem::scan_for_matching_filenames(const std::string &pattern_, match_results frame_match; if (regex_match (f, frame_match, pattern_re)) { std::string thenumber (frame_match[1].first, frame_match[1].second); - int frame = (int)strtol (thenumber.c_str(), NULL, 10); + int frame = Strutil::stoi (thenumber); matches.push_back (std::make_pair (frame, f)); } } diff --git a/src/libutil/strutil.cpp b/src/libutil/strutil.cpp index 80de761f65..56c016ebd3 100644 --- a/src/libutil/strutil.cpp +++ b/src/libutil/strutil.cpp @@ -31,12 +31,17 @@ #include #include +#include #include #include #include #include #include #include +#include +#if defined(__APPLE__) || defined(__FreeBSD__) +#include +#endif #include @@ -61,6 +66,16 @@ static std::mutex output_mutex; +// Locale-independent quickie ASCII digit and alphanum tests, good enough +// for our parsing. +inline int isupper (char c) { return c >= 'A' && c <= 'Z'; } +inline int islower (char c) { return c >= 'a' && c <= 'z'; } +inline int isalpha (char c) { return isupper(c) || islower(c); } +inline int isdigit (char c) { return c >= '0' && c <= '9'; } + + + + OIIO_NO_SANITIZE_ADDRESS const char * string_view::c_str() const @@ -694,14 +709,12 @@ Strutil::parse_int (string_view &str, int &val, bool eat) skip_whitespace (p); if (! p.size()) return false; - const char *end = p.begin(); - int v = strtol (p.begin(), (char**)&end, 10); - if (end == p.begin()) + size_t endpos = 0; + int v = Strutil::stoi (p, &endpos); + if (endpos == 0) return false; // no integer found - if (eat) { - p.remove_prefix (end-p.begin()); - str = p; - } + if (eat) + str = p.substr (endpos); val = v; return true; } @@ -715,14 +728,12 @@ Strutil::parse_float (string_view &str, float &val, bool eat) skip_whitespace (p); if (! p.size()) return false; - const char *end = p.begin(); - float v = (float) strtod (p.begin(), (char**)&end); - if (end == p.begin()) + size_t endpos = 0; + float v = Strutil::stof (p, &endpos); + if (endpos == 0) return false; // no integer found - if (eat) { - p.remove_prefix (end-p.begin()); - str = p; - } + if (eat) + str = p.substr (endpos); val = v; return true; } @@ -1032,4 +1043,277 @@ Strutil::base64_encode (string_view str) } + +// Helper: Eat the given number of chars from str, then return the next +// char, or 0 if no more chars are in str. +inline unsigned char cnext (string_view& str, int eat=1) +{ + str.remove_prefix (eat); + return OIIO_LIKELY(str.size()) ? str.front() : 0; +} + + + +int +Strutil::stoi (string_view str, size_t *pos, int base) +{ + // We roll our own stoi so that we can directly use it with a + // string_view and also hardcode it without any locale dependence. The + // system stoi/atoi/strtol/etc. needs to be null-terminated. + string_view str_orig = str; + Strutil::skip_whitespace (str); + + // c is always the next char to parse, or 0 if there's no more + unsigned char c = cnext (str, 0); // don't eat the char + + // Handle leading - or + + bool neg = (c == '-'); + if (c == '-' || c == '+') + c = cnext (str); + + // Allow "0x" to start hex number if base is 16 or 0 (any) + if ((base == 0 || base == 16) && + c == '0' && str.size() >= 1 && (str[1] == 'x' || str[1] == 'X')) { + base = 16; + c = cnext (str, 2); + } + // For "any" base, collapse to base 10 unless leading 0 means it's octal + if (base == 0) + base = c == '0' ? 8 : 10; + + // Accumulate into a 64 bit int to make overflow handling simple. + // If we ever need a str-to-int64 conversion, we'll need to be more + // careful with figuring out overflow. But the 32 bit case is easy. + int64_t acc = 0; + bool overflow = false; // Set if we overflow + bool anydigits = false; // Have we parsed any digits at all? + int64_t maxval = neg ? -int64_t(std::numeric_limits::min()) : std::numeric_limits::max(); + for ( ; OIIO_LIKELY(c); c = cnext(str)) { + if (OIIO_LIKELY(isdigit(c))) + c -= '0'; + else if (isalpha(c)) + c -= isupper(c) ? 'A' - 10 : 'a' - 10; + else { + break; // done + } + if (c >= base) + break; + acc = acc * base + c; + anydigits = true; + if (OIIO_UNLIKELY(acc > maxval)) + overflow = true; + } + if (OIIO_UNLIKELY(!anydigits)) { + str = str_orig; + } else if (OIIO_UNLIKELY(overflow)) { + acc = neg ? std::numeric_limits::min() : std::numeric_limits::max(); + } else { + if (neg) + acc = -acc; + } + if (pos) + *pos = size_t (str.data() - str_orig.data()); + return static_cast(acc); +} + + + +float +Strutil::strtof (const char *nptr, char **endptr) +{ + // Can use strtod_l on platforms that support it +#if defined (__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__) + // static initialization inside function is thread-safe by C++11 rules! + static locale_t c_loc = newlocale(LC_ALL_MASK, "C", nullptr); +# ifdef __APPLE__ + // On OSX, strtod_l is for some reason drastically faster than strtof_l. + return static_cast(strtod_l (nptr, endptr, c_loc)); +# else + return strtof_l (nptr, endptr, c_loc); +# endif +#elif defined (_WIN32) + // Windows has _strtod_l + static _locale_t c_loc = _create_locale(LC_ALL, "C"); + return static_cast(_strtod_l (nptr, endptr, c_loc)); +#else + // On platforms without strtof_l... + std::locale native; // default ctr gets current global locale + char nativepoint = std::use_facet>(native).decimal_point(); + // If the native locale uses decimal, just directly use strtof. + if (nativepoint = '.') + return strtof (nptr, endptr); + // Complex case -- CHEAT by making a copy of the string and replacing + // the decimal, then use system strtof! + std::string s (nptr); + const char* pos = strchr (nptr, pointchar); + if (pos) { + s[pos-nptr] = nativepoint; + auto d = strtof (s.c_str(), endptr); + if (endptr) + *endptr = (char *)nptr + (*endptr - s.c_str()); + return d; + } + // No decimal point at all -- use regular strtof + return strtof (s.c_str(), endptr); +#endif +} + + +double +Strutil::strtod (const char *nptr, char **endptr) +{ + // Can use strtod_l on platforms that support it +#if defined (__linux__) || defined(__APPLE__) || defined(__FreeBSD__) || defined(__FreeBSD_kernel__) + // static initialization inside function is thread-safe by C++11 rules! + static locale_t c_loc = newlocale(LC_ALL_MASK, "C", nullptr); + return strtod_l (nptr, endptr, c_loc); +#elif defined (_WIN32) + // Windows has _strtod_l + static _locale_t c_loc = _create_locale(LC_ALL, "C"); + return _strtod_l (nptr, endptr, c_loc); +#else + // On platforms without strtod_l... + std::locale native; // default ctr gets current global locale + char nativepoint = std::use_facet>(native).decimal_point(); + // If the native locale uses decimal, just directly use strtod. + if (nativepoint = '.') + return strtod (nptr, endptr); + // Complex case -- CHEAT by making a copy of the string and replacing + // the decimal, then use system strtod! + std::string s (nptr); + const char* pos = strchr (nptr, pointchar); + if (pos) { + s[pos-nptr] = nativepoint; + auto d = ::strtod (s.c_str(), endptr); + if (endptr) + *endptr = (char *)nptr + (*endptr - s.c_str()); + return d; + } + // No decimal point at all -- use regular strtod + return strtod (s.c_str(), endptr); +#endif +} + +// Notes: +// +// FreeBSD's implementation of strtod: +// https://svnweb.freebsd.org/base/stable/10/contrib/gdtoa/strtod.c?view=markup +// Python's implementation of strtod: (BSD license) +// https://hg.python.org/cpython/file/default/Python/pystrtod.c +// Julia's implementation (combo of strtod_l and Python's impl): +// https://github.com/JuliaLang/julia/blob/master/src/support/strtod.c +// MSDN documentation on Windows _strtod_l and friends: +// https://msdn.microsoft.com/en-us/library/kxsfc1ab.aspx (_strtod_l) +// https://msdn.microsoft.com/en-us/library/4zx9aht2.aspx (_create_locale) +// cppreference on locale: +// http://en.cppreference.com/w/cpp/locale/locale + + + +float +Strutil::stof (const char* s, size_t* pos) +{ + if (s) { + char* endptr; + float r = Strutil::strtof (s, &endptr); + if (endptr != s) { + if (pos) + *pos = size_t (endptr - s); + return r; + } + } + // invalid + if (pos) + *pos = 0; + return 0; +} + + +float +Strutil::stof (const std::string& s, size_t* pos) +{ + return Strutil::stof(s.c_str(), pos); +} + + +float +Strutil::stof (string_view s, size_t* pos) +{ + // string_view can't be counted on to end with a terminating null, so + // for safety, create a temporary string. This looks wasteful, but it's + // not as bad as you think -- fully compliant C++ >= 11 implementations + // will use the "short string optimization", meaning that this string + // creation will NOT need an allocation/free for most strings we expect + // to hold a text representation of a float. + return Strutil::stof (std::string(s).c_str(), pos); +} + + + +double +Strutil::stod (const char* s, size_t* pos) +{ + if (s) { + char* endptr; + double r = Strutil::strtod (s, &endptr); + if (endptr != s) { + if (pos) + *pos = size_t (endptr - s); + return r; + } + } + // invalid + if (pos) + *pos = 0; + return 0; +} + + +double +Strutil::stod (const std::string& s, size_t* pos) +{ + return Strutil::stod(s.c_str(), pos); +} + + +double +Strutil::stod (string_view s, size_t* pos) +{ + // string_view can't be counted on to end with a terminating null, so + // for safety, create a temporary string. This looks wasteful, but it's + // not as bad as you think -- fully compliant C++ >= 11 implementations + // will use the "short string optimization", meaning that this string + // creation will NOT need an allocation/free for most strings we expect + // to hold a text representation of a float. + return Strutil::stod (std::string(s).c_str(), pos); +} + + + +bool +Strutil::string_is_int (string_view s) +{ + size_t pos; + Strutil::stoi (s, &pos); + if (pos) { // skip remaining whitespace + s.remove_prefix (pos); + Strutil::skip_whitespace (s); + } + return pos && s.empty(); // consumed the whole string +} + + +bool +Strutil::string_is_float (string_view s) +{ + size_t pos; + Strutil::stof (s, &pos); + if (pos) { // skip remaining whitespace + s.remove_prefix (pos); + Strutil::skip_whitespace (s); + } + return pos && s.empty(); // consumed the whole string +} + + OIIO_NAMESPACE_END diff --git a/src/libutil/strutil_test.cpp b/src/libutil/strutil_test.cpp index c583432b5e..45cd70f158 100644 --- a/src/libutil/strutil_test.cpp +++ b/src/libutil/strutil_test.cpp @@ -30,6 +30,7 @@ #include +#include #include #include @@ -353,41 +354,123 @@ void test_replace () void test_conversion () { std::cout << "Testing string_is, string_from conversions\n"; - OIIO_CHECK_EQUAL (Strutil::string_is("142"), true); - OIIO_CHECK_EQUAL (Strutil::string_is("142.0"), false); - OIIO_CHECK_EQUAL (Strutil::string_is(""), false); - OIIO_CHECK_EQUAL (Strutil::string_is("foo"), false); - OIIO_CHECK_EQUAL (Strutil::string_is("142x"), false); - OIIO_CHECK_EQUAL (Strutil::string_is(" 142"), true); - OIIO_CHECK_EQUAL (Strutil::string_is("x142"), false); - - OIIO_CHECK_EQUAL (Strutil::string_is("142"), true); - OIIO_CHECK_EQUAL (Strutil::string_is("142.0"), true); - OIIO_CHECK_EQUAL (Strutil::string_is(""), false); - OIIO_CHECK_EQUAL (Strutil::string_is("foo"), false); - OIIO_CHECK_EQUAL (Strutil::string_is("142x"), false); - OIIO_CHECK_EQUAL (Strutil::string_is(" 142"), true); - OIIO_CHECK_EQUAL (Strutil::string_is("x142"), false); - - OIIO_CHECK_EQUAL (Strutil::from_string("hi"), 0); - OIIO_CHECK_EQUAL (Strutil::from_string("123"), 123); - OIIO_CHECK_EQUAL (Strutil::from_string("-123"), -123); - OIIO_CHECK_EQUAL (Strutil::from_string(" 123 "), 123); - OIIO_CHECK_EQUAL (Strutil::from_string("123.45"), 123); - - OIIO_CHECK_EQUAL (Strutil::from_string("hi"), unsigned(0)); - OIIO_CHECK_EQUAL (Strutil::from_string("123"), unsigned(123)); - OIIO_CHECK_EQUAL (Strutil::from_string("-123"), unsigned(-123)); - OIIO_CHECK_EQUAL (Strutil::from_string(" 123 "), unsigned(123)); - OIIO_CHECK_EQUAL (Strutil::from_string("123.45"), unsigned(123)); - - OIIO_CHECK_EQUAL (Strutil::from_string("hi"), 0.0f); - OIIO_CHECK_EQUAL (Strutil::from_string("123"), 123.0f); - OIIO_CHECK_EQUAL (Strutil::from_string("-123"), -123.0f); - OIIO_CHECK_EQUAL (Strutil::from_string("123.45"), 123.45f); - OIIO_CHECK_EQUAL (Strutil::from_string(" 123.45 "), 123.45f); - OIIO_CHECK_EQUAL (Strutil::from_string("123.45+12"), 123.45f); - OIIO_CHECK_EQUAL (Strutil::from_string("1.2345e+2"), 123.45f); + size_t pos; + + OIIO_CHECK_EQUAL (Strutil::string_is_int("142"), true); + OIIO_CHECK_EQUAL (Strutil::string_is_int("-142"), true); + OIIO_CHECK_EQUAL (Strutil::string_is_int("+142"), true); + OIIO_CHECK_EQUAL (Strutil::string_is_int("142.0"), false); + OIIO_CHECK_EQUAL (Strutil::string_is_int(""), false); + OIIO_CHECK_EQUAL (Strutil::string_is_int(" "), false); + OIIO_CHECK_EQUAL (Strutil::string_is_int("foo"), false); + OIIO_CHECK_EQUAL (Strutil::string_is_int("142x"), false); + OIIO_CHECK_EQUAL (Strutil::string_is_int(" 142"), true); + OIIO_CHECK_EQUAL (Strutil::string_is_int("142 "), true); + OIIO_CHECK_EQUAL (Strutil::string_is_int("x142"), false); + + OIIO_CHECK_EQUAL (Strutil::string_is_float("142"), true); + OIIO_CHECK_EQUAL (Strutil::string_is_float("142.0"), true); + OIIO_CHECK_EQUAL (Strutil::string_is_float(""), false); + OIIO_CHECK_EQUAL (Strutil::string_is_float(" "), false); + OIIO_CHECK_EQUAL (Strutil::string_is_float("foo"), false); + OIIO_CHECK_EQUAL (Strutil::string_is_float("142x"), false); + OIIO_CHECK_EQUAL (Strutil::string_is_float(" 142"), true); + OIIO_CHECK_EQUAL (Strutil::string_is_float(" 142 "), true); + OIIO_CHECK_EQUAL (Strutil::string_is_float(" 142.0 "), true); + OIIO_CHECK_EQUAL (Strutil::string_is_float("x142"), false); + + // Note: we don't test string_is<> separately because it's just + // implemented directly as calls to string_is_{int,float}. + + OIIO_CHECK_EQUAL (Strutil::stoi("hi"), 0); + OIIO_CHECK_EQUAL (Strutil::stoi(" "), 0); + OIIO_CHECK_EQUAL (Strutil::stoi("123"), 123); + OIIO_CHECK_EQUAL (Strutil::stoi("-123"), -123); + OIIO_CHECK_EQUAL (Strutil::stoi("+123"), 123); + OIIO_CHECK_EQUAL (Strutil::stoi(" 123 "), 123); + OIIO_CHECK_EQUAL (Strutil::stoi("123.45"), 123); + OIIO_CHECK_EQUAL (Strutil::stoi("12345678901234567890"), std::numeric_limits::max()); + OIIO_CHECK_EQUAL (Strutil::stoi("-12345678901234567890"), std::numeric_limits::min()); + + OIIO_CHECK_EQUAL (Strutil::stoi("hi", &pos), 0); + OIIO_CHECK_EQUAL (pos, 0); + OIIO_CHECK_EQUAL (Strutil::stoi(" ", &pos), 0); + OIIO_CHECK_EQUAL (pos, 0); + OIIO_CHECK_EQUAL (Strutil::stoi("123", &pos), 123); + OIIO_CHECK_EQUAL (pos, 3); + OIIO_CHECK_EQUAL (Strutil::stoi("-123", &pos), -123); + OIIO_CHECK_EQUAL (pos, 4); + OIIO_CHECK_EQUAL (Strutil::stoi(" 123 ", &pos), 123); + OIIO_CHECK_EQUAL (pos, 4); + OIIO_CHECK_EQUAL (Strutil::stoi("123.45", &pos), 123); + OIIO_CHECK_EQUAL (pos, 3); + +#if 0 + // Make sure it's correct for EVERY value. This takes too long to do as + // part of unit tests, but I assure you that I did it once to confirm. + for (int64_t i = std::numeric_limits::min(); i <= std::numeric_limits::max(); ++i) + OIIO_CHECK_EQUAL (Strutil::stoi(Strutil::format("%d",i)), i); +#endif + + OIIO_CHECK_EQUAL (Strutil::stoui("hi"), unsigned(0)); + OIIO_CHECK_EQUAL (Strutil::stoui(" "), unsigned(0)); + OIIO_CHECK_EQUAL (Strutil::stoui("123"), unsigned(123)); + OIIO_CHECK_EQUAL (Strutil::stoui("-123"), unsigned(-123)); + OIIO_CHECK_EQUAL (Strutil::stoui(" 123 "), unsigned(123)); + OIIO_CHECK_EQUAL (Strutil::stoui("123.45"), unsigned(123)); + + OIIO_CHECK_EQUAL (Strutil::stof("hi"), 0.0f); + OIIO_CHECK_EQUAL (Strutil::stof(" "), 0.0f); + OIIO_CHECK_EQUAL (Strutil::stof("123"), 123.0f); + OIIO_CHECK_EQUAL (Strutil::stof("-123"), -123.0f); + OIIO_CHECK_EQUAL (Strutil::stof("123.45"), 123.45f); + OIIO_CHECK_EQUAL (Strutil::stof("123.45xyz"), 123.45f); + OIIO_CHECK_EQUAL (Strutil::stof(" 123.45 "), 123.45f); + OIIO_CHECK_EQUAL (Strutil::stof("123.45+12"), 123.45f); + OIIO_CHECK_EQUAL (Strutil::stof("1.2345e+2"), 123.45f); + + OIIO_CHECK_EQUAL (Strutil::stof("hi", &pos), 0.0f); + OIIO_CHECK_EQUAL (pos, 0); + OIIO_CHECK_EQUAL (Strutil::stof(" ", &pos), 0.0f); + OIIO_CHECK_EQUAL (pos, 0); + OIIO_CHECK_EQUAL (Strutil::stof("123", &pos), 123.0f); + OIIO_CHECK_EQUAL (pos, 3); + OIIO_CHECK_EQUAL (Strutil::stof("-123", &pos), -123.0f); + OIIO_CHECK_EQUAL (pos, 4); + OIIO_CHECK_EQUAL (Strutil::stof("123.45", &pos), 123.45f); + OIIO_CHECK_EQUAL (pos, 6); + OIIO_CHECK_EQUAL (Strutil::stof("123.45xyz", &pos), 123.45f); + OIIO_CHECK_EQUAL (pos, 6); + OIIO_CHECK_EQUAL (Strutil::stof(" 123.45 ", &pos), 123.45f); + OIIO_CHECK_EQUAL (pos, 7); + OIIO_CHECK_EQUAL (Strutil::stof("123.45+12", &pos), 123.45f); + OIIO_CHECK_EQUAL (pos, 6); + OIIO_CHECK_EQUAL (Strutil::stof("1.2345e2", &pos), 123.45f); + OIIO_CHECK_EQUAL (pos, 8); + // stress case! + OIIO_CHECK_EQUAL (Strutil::stof("100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001E-200"), 1.0f); + OIIO_CHECK_EQUAL (Strutil::stof("0.00000000000000000001"), 1.0e-20f); + + // Note: we don't test from_strings<> separately because it's just + // implemented directly as calls to stoi, stoui, stof. + + Benchmarker bench; + bench.indent (2); + bench.units (Benchmarker::Unit::ns); + const char* numcstr = "123.45"; + std::string numstring (numcstr); + bench ("get default locale", [](){ std::locale loc; DoNotOptimize (loc); }); + bench ("ref classic locale", [](){ DoNotOptimize (std::locale::classic()); }); + bench ("std atoi", [&](){ DoNotOptimize(atoi(numcstr));}); + bench ("Strutil::stoi(string) ", [&](){ return DoNotOptimize(Strutil::stoi(numstring)); }); + bench ("Strutil::stoi(char*) ", [&](){ return DoNotOptimize(Strutil::stoi(numcstr)); }); + bench ("std atof", [&](){ DoNotOptimize(atof(numcstr));}); + bench ("std strtod", [&](){ DoNotOptimize(::strtod(numcstr, nullptr));}); + bench ("Strutil::from_string", [&](){ DoNotOptimize(Strutil::from_string(numstring));}); + bench ("Strutil::stof(string) - locale-independent", [&](){ return DoNotOptimize(Strutil::stof(numstring)); }); + bench ("Strutil::stof(char*) - locale-independent", [&](){ return DoNotOptimize(Strutil::stof(numcstr)); }); + bench ("Strutil::stof(string_view) - locale-independent", [&](){ return DoNotOptimize(Strutil::stof(string_view(numstring))); }); + bench ("locale switch (to classic)", [&](){ std::locale::global (std::locale::classic()); }); } @@ -616,6 +699,25 @@ void test_parse () +void +test_locale () +{ + std::cout << "Testing float conversion + locale\n"; + std::locale oldloc = std::locale::global(std::locale::classic()); // save original locale + std::locale::global (std::locale("fr_FR.UTF-8")); + const char* numcstr = "123.45"; + std::string numstring (numcstr); + std::cout << "safe float convert (C locale) " << numcstr << " = " << Strutil::stof(numcstr) << "\n"; + OIIO_CHECK_EQUAL_APPROX (Strutil::stof(numcstr), 123.45f); + std::cout << "unsafe float convert (default locale) " << numcstr << " = " << atof(numcstr) << "\n"; + OIIO_CHECK_EQUAL_APPROX (atof(numcstr), 123.0f); + // Verify that Strutil::format does the right thing, even when in a + // comma-based locale. + OIIO_CHECK_EQUAL (Strutil::format ("%g", 123.45f), "123.45"); + std::locale::global (oldloc); // restore +} + + void test_float_formatting () @@ -668,6 +770,7 @@ main (int argc, char *argv[]) test_safe_strcpy (); test_string_view (); test_parse (); + test_locale (); // test_float_formatting (); return unit_test_failures; diff --git a/src/libutil/ustring.cpp b/src/libutil/ustring.cpp index 90663355ce..31ed368481 100644 --- a/src/libutil/ustring.cpp +++ b/src/libutil/ustring.cpp @@ -406,6 +406,7 @@ ustring::getstats (bool verbose) { UstringTable &table (ustring_table()); std::ostringstream out; + out.imbue (std::locale::classic()); // Force "C" locale with '.' decimal size_t n_l = table.get_num_lookups(); size_t n_e = table.get_num_entries(); size_t mem = table.get_memory_usage(); diff --git a/src/maketx/maketx.cpp b/src/maketx/maketx.cpp index 9b740d64b7..ee59b34360 100644 --- a/src/maketx/maketx.cpp +++ b/src/maketx/maketx.cpp @@ -446,6 +446,10 @@ main (int argc, char *argv[]) { Timer alltimer; + // Globally force classic "C" locale, and turn off all formatting + // internationalization, for the entire maketx application. + std::locale::global (std::locale::classic()); + ImageSpec configspec; Filesystem::convert_native_arguments (argc, (const char **)argv); getargs (argc, argv, configspec); diff --git a/src/oiiotool/oiiotool.cpp b/src/oiiotool/oiiotool.cpp index c79e409208..66d66648af 100644 --- a/src/oiiotool/oiiotool.cpp +++ b/src/oiiotool/oiiotool.cpp @@ -1325,34 +1325,20 @@ set_input_attribute (int argc, const char *argv[]) return 0; } - // Does it seem to be an int, or did the caller explicitly request - // that it be set as an int? - char *p = NULL; - int i = strtol (value.c_str(), &p, 10); - while (*p && isspace(*p)) - ++p; - if ((! *p && type == TypeDesc::UNKNOWN) || type == TypeDesc::INT) { - // int conversion succeeded and accounted for the whole string -- - // so set an int attribute. - ot.input_config.attribute (attribname, i); - return 0; - } - - // Does it seem to be a float, or did the caller explicitly request - // that it be set as a float? - p = NULL; - float f = (float)strtod (value.c_str(), &p); - while (*p && isspace(*p)) - ++p; - if ((! *p && type == TypeDesc::UNKNOWN) || type == TypeDesc::FLOAT) { - // float conversion succeeded and accounted for the whole string -- - // so set a float attribute. - ot.input_config.attribute (attribname, f); - return 0; + if (type == TypeInt || + (type == TypeUnknown && Strutil::string_is_int(value))) { + // Does it seem to be an int, or did the caller explicitly request + // that it be set as an int? + ot.input_config.attribute (attribname, Strutil::stoi(value)); + } else if (type == TypeFloat || + (type == TypeUnknown && Strutil::string_is_float(value))) { + // Does it seem to be a float, or did the caller explicitly request + // that it be set as a float? + ot.input_config.attribute (attribname, Strutil::stof(value)); + } else { + // Otherwise, set it as a string attribute + ot.input_config.attribute (attribname, value); } - - // Otherwise, set it as a string attribute - ot.input_config.attribute (attribname, value); return 0; } @@ -1478,38 +1464,28 @@ OiioTool::set_attribute (ImageRecRef img, string_view attribname, return true; } - // Does it seem to be an int, or did the caller explicitly request - // that it be set as an int? - char *p = NULL; - int i = strtol (value.c_str(), &p, 10); - while (*p && isspace(*p)) - ++p; - if ((! *p && type == TypeDesc::UNKNOWN) || type == TypeDesc::INT) { - // int conversion succeeded and accounted for the whole string -- - // so set an int attribute. + if (type == TypeInt || + (type == TypeUnknown && Strutil::string_is_int(value))) { + // Does it seem to be an int, or did the caller explicitly request + // that it be set as an int? + int v = Strutil::stoi(value); return apply_spec_mod (*img, do_set_any_attribute, - std::pair(attribname,i), + std::pair(attribname,v), allsubimages); - } - - // Does it seem to be a float, or did the caller explicitly request - // that it be set as a float? - p = NULL; - float f = (float)strtod (value.c_str(), &p); - while (*p && isspace(*p)) - ++p; - if ((! *p && type == TypeDesc::UNKNOWN) || type == TypeDesc::FLOAT) { - // float conversion succeeded and accounted for the whole string -- - // so set a float attribute. + } else if (type == TypeFloat || + (type == TypeUnknown && Strutil::string_is_float(value))) { + // Does it seem to be a float, or did the caller explicitly request + // that it be set as a float? + float v = Strutil::stof(value); return apply_spec_mod (*img, do_set_any_attribute, - std::pair(attribname,f), + std::pair(attribname,v), + allsubimages); + } else { + // Otherwise, set it as a string attribute + return apply_spec_mod (*img, do_set_any_attribute, + std::pair(attribname,value), allsubimages); } - - // Otherwise, set it as a string attribute - return apply_spec_mod (*img, do_set_any_attribute, - std::pair(attribname,value), - allsubimages); } @@ -3789,7 +3765,7 @@ action_mosaic (int argc, const char *argv[]) std::map options; options["pad"] = "0"; ot.extract_options (options, command); - int pad = strtol (options["pad"].c_str(), NULL, 10); + int pad = Strutil::stoi (options["pad"]); ImageSpec Rspec (ximages*widest + (ximages-1)*pad, yimages*highest + (yimages-1)*pad, @@ -4016,7 +3992,7 @@ action_clamp (int argc, const char *argv[]) ot.extract_options (options, command); Strutil::extract_from_list_string (min, options["min"]); Strutil::extract_from_list_string (max, options["max"]); - bool clampalpha01 = strtol (options["clampalpha"].c_str(), NULL, 10) != 0; + bool clampalpha01 = Strutil::stoi (options["clampalpha"]); for (int m = 0, miplevels=R->miplevels(s); m < miplevels; ++m) { ImageBuf &Rib ((*R)(s,m)); @@ -5420,6 +5396,10 @@ main (int argc, char *argv[]) _set_output_format (_TWO_DIGIT_EXPONENT); #endif + // Globally force classic "C" locale, and turn off all formatting + // internationalization, for the entire oiiotool application. + std::locale::global (std::locale::classic()); + ot.imagecache = ImageCache::create (false); ASSERT (ot.imagecache); ot.imagecache->attribute ("forcefloat", 1); diff --git a/src/rla.imageio/rlainput.cpp b/src/rla.imageio/rlainput.cpp index 672ae894c4..126f666b6f 100644 --- a/src/rla.imageio/rlainput.cpp +++ b/src/rla.imageio/rlainput.cpp @@ -392,7 +392,7 @@ RLAInput::seek_subimage (int subimage, int miplevel, ImageSpec &newspec) } } - float aspect = atof (m_rla.AspectRatio); + float aspect = Strutil::stof (m_rla.AspectRatio); if (aspect > 0.f) m_spec.attribute ("PixelAspectRatio", aspect);