Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code

["Daniel P. Berrange" <berrange@redhat.com>]

On Wed, Aug 26, 2015 at 11:13:00AM +0200, Alberto Garcia wrote:
> On Tue 25 Aug 2015 09:54:42 AM CEST, Markus Armbruster wrote:
> 
> >> (D) Run in a controlled mixed locale
> >>    GTK runs completely in the locale determined by setlocale() (since it
> >> never has to display raw JSON)
> >>    We fix our JSON output code to use thread-specific locale
> >> manipulations to (temporarily) switch to C locale before printing JSON
> >>
> >> that way, GTK still shows full German formatting for any localized
> >> message in the GUI that happens to print numbers, but the JSON output
> >> (which is independent of the GUI) also behaves correctly as a C-only
> >> entity.
> >
> > Switching back to C locale whenever some unwanted locale-dependency
> > breaks the code is problematic, because it involves finding all the
> > places that break, iteratively (euphemism for "we debug one breakage
> > after the other, adding temporary locale switches as we go).
> 
> For some cases we probably don't even need to switch the locale. For the
> JSON case cannot we just easily convert the QFloat into a string
> ourselves without using printf's "%f" ?

FWIW, libvirt had this exact same problem with formatting doubles for
JSON while non-C locales are in effect. We used the glibc thread-local
locale support, and fallback to some sick string munging in non-glibc
case:


/* In case thread-safe locales are available */
#if HAVE_NEWLOCALE

static locale_t virLocale;

static int
virLocaleOnceInit(void)
{
    virLocale = newlocale(LC_ALL_MASK, "C", (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);
    uselocale(old_loc);

#else

    char *radix, *tmp;
    struct lconv *lc;

    if ((ret = virVasprintf(strp, "%lf", number) < 0)
        goto error;

    lc = localeconv();
    radix = lc->decimal_point;
    tmp = strstr(*strp, radix);
    if (tmp) {
        *tmp = '.';
        if (strlen(radix) > 1)
            memmove(tmp + 1, tmp + strlen(radix), strlen(*strp) - (tmp - str));
    }

#endif /* HAVE_NEWLOCALE */
 error:
    return ret;
}


> That doesn't invalidate however your point.
> 
> > I'd feel much better about confining GTK in its own thread, and
> > setting only that thread's locale.
> 
> I haven't digged much into that part of the QEMU code, but if my
> assumptions are true I think we would need at least:
> 
> - A separate GMainContext with its own main loop
> - Making sure that all the code that touches the UI runs from that
>   thread.
> 
> This is in principle possible, but I fear that we might be opening a can
> of worms.

Agreed, my experiance is that you should always run GTK in the main
thread and never try todo anything more clever with threads + GTK.
It has always ended in tears for me - even if you think you have it
working on your system, you'll inevitably find a different version
of GTK where it has broken. It just isn't worth the pain to try to
run anything GTK related in a non-main thread.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|