[libvirt] [PATCH v3.1] json: fix interface locale dependency

[Eric Blake]

On 08/12/2012 01:55 AM, Martin Kletzander wrote:
> libvirt creates invalid commands if wrong locale is selected. For
> example with locale that uses comma as a decimal point, JSON commands
> created with decimal numbers are invalid because comma separates the
> entries in JSON. Fortunately even when decimal point is affected,
> thousands grouping is not, because for grouping to be enabled with
> *printf, there has to be an apostrophe flag specified (and supported).
> 
> This patch adds specific internal function for converting doubles to
> strings with C locale.
> ---
>  bootstrap.conf           |    1 +
>  configure.ac             |    2 +-
>  src/libvirt_private.syms |    1 +
>  src/util/json.c          |    2 +-
>  src/util/util.c          |   63 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/util.h          |    2 +
>  6 files changed, 69 insertions(+), 2 deletions(-)

> +++ b/configure.ac
> @@ -133,7 +133,7 @@ dnl Availability of various common functions (non-fatal if missing),
>  dnl and various less common threadsafe functions
>  AC_CHECK_FUNCS_ONCE([cfmakeraw geteuid getgid getgrnam_r getmntent_r \
>    getpwuid_r getuid initgroups kill mmap posix_fallocate posix_memalign \
> -  regexec sched_getaffinity])
> +  regexec sched_getaffinity newlocale])

Insert in sorted order.

> 
> +/* In case thread-safe locales are available */
> +#if HAVE_NEWLOCALE
> +
> +locale_t virLocale;

Mark this static.

> +
> +static int
> +virLocaleOnceInit(void)
> +{
> +    virLocale = newlocale(LC_ALL_MASK, "C", NULL);

NULL is technically incorrect here; locale_t is not necessarily a
pointer type.  POSIX says the third argument should be '(locale_t)0'.

> +    if (!virLocale)
> +        return -1;
> +    return 0;
> +}
> +
> +VIR_ONCE_GLOBAL_INIT(virLocale)
> +#endif
> +
> +/**
> + * virDoubleToStr
> + *
> + * converts double to string with C locale (thread-safe).
> + *
> + * Returns -1 on error, size of the string otherwise.
> + */
> +int
> +virDoubleToStr(char **strp, double number)
> +{
> +    int ret = -1;
> +
> +#if HAVE_NEWLOCALE
> +
> +    locale_t old_loc;
> +
> +    if (virLocaleInitialize() < 0)
> +        goto error;
> +
> +    old_loc = uselocale(virLocale);
> +    ret = virAsprintf(strp, "%lf", number);

Technically, %f and %lf are identical.  I also wonder if we should be
favoring %g instead of %f here, or if we need to care about the rounding
errors introduced by the fact that the default precision of %f is
smaller than the actual precision in the double value being converted.
But as this is the pre-existing format, changing it would deserve a
separate commit, so it does not impact this commit.

> +++ b/src/util/util.h
> @@ -209,6 +209,8 @@ char *virStrcpy(char *dest, const char *src, size_t destbytes)
>      ATTRIBUTE_RETURN_CHECK;
>  # define virStrcpyStatic(dest, src) virStrcpy((dest), (src), sizeof(dest))
> 
> +int virDoubleToStr(char **strp, double number) ATTRIBUTE_NONNULL(1);
> +

I'd also add ATTRIBUTE_RETURN_CHECK.

Of the two alternatives, I prefer this one for now.  If we ever have
need for further enhancements down the road, we can add them at that
time.  No need to add code that will be unused by being more flexible
than our current needs.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120813/29e06e12/attachment-0001.sig>