[libcamera-devel] [PATCH v1 1/2] base: utils: Add and use strtod() helper

[Laurent Pinchart]

Hi Jacopo,

On Tue, Jan 10, 2023 at 09:10:52AM +0100, Jacopo Mondi wrote:
> Hi Laurent
> 
>   I presume set_locale()
> https://en.cppreference.com/w/cpp/locale/setlocale
> 
> doesn't help here, right ?

It doesn't, as that changes the current locale, which we can't do as a
library.

> On Sun, Jan 08, 2023 at 11:43:56PM +0200, Laurent Pinchart via libcamera-devel wrote:
> > The strtod() function is locale-dependent, and thus ill-suited to parse
> > numbers coming from, for instance, YAML files. The YamlObject class uses
> > strtod_l() to fix that issue, but that function is not available with
> > all libc implementations. Correctly handling this problem is becoming
> > out of scope for the YamlObject class.
> >
> > As a first step, add a strtod() helper function in the utils namespace
> > that copies the implementation from YamlObject, and use it in
> > YamlObject. The core issue will then be fixed in utils::strtod().
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/base/utils.h |  2 ++
> >  src/libcamera/base/utils.cpp   | 46 ++++++++++++++++++++++++++++++++++
> >  src/libcamera/yaml_parser.cpp  | 34 +------------------------
> >  3 files changed, 49 insertions(+), 33 deletions(-)
> >
> > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> > index eb7bcdf4c173..37d9af609ec7 100644
> > --- a/include/libcamera/base/utils.h
> > +++ b/include/libcamera/base/utils.h
> > @@ -367,6 +367,8 @@ decltype(auto) abs_diff(const T &a, const T &b)
> >  		return a - b;
> >  }
> >
> > +double strtod(const char *__restrict nptr, char **__restrict endptr);
> > +
> >  } /* namespace utils */
> >
> >  #ifndef __DOXYGEN__
> > diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> > index 6a307940448e..4a239427a4d9 100644
> > --- a/src/libcamera/base/utils.cpp
> > +++ b/src/libcamera/base/utils.cpp
> > @@ -8,6 +8,7 @@
> >  #include <libcamera/base/utils.h>
> >
> >  #include <iomanip>
> > +#include <locale.h>
> >  #include <sstream>
> >  #include <stdlib.h>
> >  #include <string.h>
> > @@ -463,6 +464,51 @@ std::string toAscii(const std::string &str)
> >   * \a b
> >   */
> >
> > +namespace {
> > +
> > +/*
> > + * RAII wrapper around locale_t instances, to support global locale instances
> > + * without leaking memory.
> > + */
> > +class Locale
> > +{
> > +public:
> > +	Locale(const char *locale)
> > +	{
> > +		locale_ = newlocale(LC_ALL_MASK, locale, static_cast<locale_t>(0));
> > +	}
> > +
> > +	~Locale()
> > +	{
> > +		freelocale(locale_);
> > +	}
> > +
> > +	locale_t locale() { return locale_; }
> > +
> > +private:
> > +	locale_t locale_;
> > +};
> > +
> > +Locale cLocale("C");
> > +
> > +} /* namespace */
> > +
> > +/**
> > + * \brief Convert a string to a double independently of the current locale
> > + * \param[in] nptr The string to convert
> > + * \param[out] endptr Pointer to trailing portion of the string after conversion
> > + *
> > + * This function is a locale-independent version of the std::strtod() function.
> > + * It behaves as the standard function, but uses the "C" locale instead of the
> > + * current locale.
> > + *
> > + * \return The converted value, if any, or 0.0 if the conversion failed.
> > + */
> > +double strtod(const char *__restrict nptr, char **__restrict endptr)
> > +{
> > +	return strtod_l(nptr, endptr, cLocale.locale());
> > +}
> > +
> >  } /* namespace utils */
> >
> >  #ifndef __DOXYGEN__
> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
> > index 2806c591f75d..153a6d53c3f9 100644
> > --- a/src/libcamera/yaml_parser.cpp
> > +++ b/src/libcamera/yaml_parser.cpp
> > @@ -31,38 +31,6 @@ 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 system 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_;
> > -};
> > -
> > -Locale yamlLocale("C");
> > -
> >  } /* namespace */
> >
> >  /**
> > @@ -315,7 +283,7 @@ std::optional<double> YamlObject::get() const
> >  	char *end;
> >
> >  	errno = 0;
> > -	double value = strtod_l(value_.c_str(), &end, yamlLocale.locale());
> > +	double value = utils::strtod(value_.c_str(), &end);
> >
> >  	if ('\0' != *end || errno == ERANGE)
> >  		return std::nullopt;

-- 
Regards,

Laurent Pinchart