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

[Jacopo Mondi]

Hi Laurent

  I presume set_locale()
https://en.cppreference.com/w/cpp/locale/setlocale

doesn't help here, right ?

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
>