| Message ID | 20221230002309.3135078-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Fri, Dec 30, 2022 at 12:23:09AM +0000, Kieran Bingham wrote: > When parsing configuration files on systems with differing locales, the > use of strtod can produce different results, or in the worst case - fail > to parse expected values. There's something missing here: Fix this by using strtod_l() instead. To avoid constructing and destructing a locale_t instance for every too to strtod_l(), create an RAII class that wraps the locale_t and use it to provide a global "C" locale. > Provide an RAII implementation to construct a locale specific to the > expected mappings for configuration files provided by libcamera. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=174 > Bug: https://github.com/raspberrypi/libcamera/issues/29 > Reported-by: https://github.com/kralo > Reported-by: Hannes Winkler <hanneswinkler2000@web.de> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > v2: > - use const char * over std::string > - use C++ static casts > - fix blank lines > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/yaml_parser.cpp | 34 +++++++++++++++++++++++++++++++++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp > index d8a7c2f9250f..c7fd80cdea1b 100644 > --- a/src/libcamera/yaml_parser.cpp > +++ b/src/libcamera/yaml_parser.cpp > @@ -31,6 +31,38 @@ namespace { > /* Empty static YamlObject as a safe result for invalid operations */ > static const YamlObject empty; > > +/* > + * Construct a global RAII locale for use by all YAML parser instances to > + * ensure consistency when parsing configuration files and types regardless of > + * the host Locale configuration. s/host/system/ s/Locale/locale/ > + * > + * For more information see: > + * - https://bugs.libcamera.org/show_bug.cgi?id=174 > + */ > +class Locale > +{ > +public: > + Locale(const char *locale) > + { > + locale_ = newlocale(LC_ALL_MASK, locale, static_cast<locale_t>(0)); > + if (locale_ == static_cast<locale_t>(0)) > + LOG(YamlParser, Fatal) > + << "Failed to construct a locale"; > + } > + > + ~Locale() > + { > + freelocale(locale_); > + } > + > + locale_t locale() { return locale_; } > + > +private: > + locale_t locale_; > +}; > + > +static Locale yamlLocale("C"); You can drop static as you're in an anonymous namespace. All these issues are minor, no need to post a v3 just for this. > + > } /* namespace */ > > /** > @@ -283,7 +315,7 @@ std::optional<double> YamlObject::get() const > char *end; > > errno = 0; > - double value = std::strtod(value_.c_str(), &end); > + double value = strtod_l(value_.c_str(), &end, yamlLocale.locale()); > > if ('\0' != *end || errno == ERANGE) > return std::nullopt;
diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp index d8a7c2f9250f..c7fd80cdea1b 100644 --- a/src/libcamera/yaml_parser.cpp +++ b/src/libcamera/yaml_parser.cpp @@ -31,6 +31,38 @@ namespace { /* Empty static YamlObject as a safe result for invalid operations */ static const YamlObject empty; +/* + * Construct a global RAII locale for use by all YAML parser instances to + * ensure consistency when parsing configuration files and types regardless of + * the host Locale configuration. + * + * For more information see: + * - https://bugs.libcamera.org/show_bug.cgi?id=174 + */ +class Locale +{ +public: + Locale(const char *locale) + { + locale_ = newlocale(LC_ALL_MASK, locale, static_cast<locale_t>(0)); + if (locale_ == static_cast<locale_t>(0)) + LOG(YamlParser, Fatal) + << "Failed to construct a locale"; + } + + ~Locale() + { + freelocale(locale_); + } + + locale_t locale() { return locale_; } + +private: + locale_t locale_; +}; + +static Locale yamlLocale("C"); + } /* namespace */ /** @@ -283,7 +315,7 @@ std::optional<double> YamlObject::get() const char *end; errno = 0; - double value = std::strtod(value_.c_str(), &end); + double value = strtod_l(value_.c_str(), &end, yamlLocale.locale()); if ('\0' != *end || errno == ERANGE) return std::nullopt;