[libcamera-devel] [PATCH v1 2/2] libcamera: base: utils: Support C libraries lacking locale support

[Jacopo Mondi]

Hi Laurent

On Sun, Jan 08, 2023 at 11:43:57PM +0200, Laurent Pinchart via libcamera-devel wrote:
> Not all C libraries include support for locale objects (locale_t) and
> the strto*_l() family of functions. A notable example is uClibc that can
> be compiled with a hardcoded "C" locale. Compilation then fails as the
> newlocale(), freelocale() and strtod_l() functions are not defined.
>
> Fix the compilation breakage by checking for the availability of
> newlocale(), and fall back to strtod() when the function isn't
> available. This may not lead to the correct result if support for locale
> objects isn't available and the locale isn't hardcoded to "C", but that
> is such a corner case that we will likely never encounter it.
>
> Fixes: e8ae254970cf ("libcamera: yaml_parser: Use C locale")
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  meson.build                  |  4 ++++
>  src/libcamera/base/utils.cpp | 12 ++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index e86673dd5c0c..7dcd34d3e6b3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -72,6 +72,10 @@ if cc.has_header_symbol('unistd.h', 'issetugid')
>      config_h.set('HAVE_ISSETUGID', 1)
>  endif
>
> +if cc.has_header_symbol('locale.h', 'newlocale', prefix : '#define _GNU_SOURCE')
> +    config_h.set('HAVE_NEWLOCALE', 1)
> +endif
> +
>  if cc.has_header_symbol('stdlib.h', 'secure_getenv', prefix : '#define _GNU_SOURCE')
>      config_h.set('HAVE_SECURE_GETENV', 1)
>  endif
> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> index 4a239427a4d9..ac3e1311995e 100644
> --- a/src/libcamera/base/utils.cpp
> +++ b/src/libcamera/base/utils.cpp
> @@ -464,6 +464,8 @@ std::string toAscii(const std::string &str)
>   * \a b
>   */
>
> +#if HAVE_NEWLOCALE
> +

Just bikeshedding on the NEWLOCALE symbol. "New" to implies there's an
old vs new version, while you're just here checking that locale is
supported. Maybe just HAVE_LOCALE ?

Apart from this:
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

Thanks
  j

>  namespace {
>
>  /*
> @@ -493,6 +495,8 @@ Locale cLocale("C");
>
>  } /* namespace */
>
> +#endif /* HAVE_NEWLOCALE */
> +
>  /**
>   * \brief Convert a string to a double independently of the current locale
>   * \param[in] nptr The string to convert
> @@ -506,7 +510,15 @@ Locale cLocale("C");
>   */
>  double strtod(const char *__restrict nptr, char **__restrict endptr)
>  {
> +#if HAVE_NEWLOCALE
>  	return strtod_l(nptr, endptr, cLocale.locale());
> +#else
> +	/*
> +	 * If the libc implementation doesn't provide locale object support,
> +	 * assume that strtod() is locale-independent.
> +	 */
> +	return strtod(nptr, endptr);
> +#endif
>  }
>
>  } /* namespace utils */
> --
> Regards,
>
> Laurent Pinchart
>