* [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
@ 2015-08-18 23:57 Alberto Garcia
2015-08-24 8:58 ` Daniel P. Berrange
2015-08-24 10:05 ` Markus Armbruster
0 siblings, 2 replies; 17+ messages in thread
From: Alberto Garcia @ 2015-08-18 23:57 UTC (permalink / raw)
To: qemu-devel
We have this code in qjson.c to produce JSON from a QFloat:
QFloat *val = qobject_to_qfloat(obj);
char buffer[1024];
int len;
len = snprintf(buffer, sizeof(buffer), "%f", qfloat_get_double(val));
while (len > 0 && buffer[len - 1] == '0') {
len--;
}
The problem here is that the output of snprintf() is locale-dependent,
so depending on the locale we might get a ',' as decimal separator.
That's not allowed by JSON and it actually breaks scripts/qmp/qmp.
This seems to happen because of GTK+ calling setlocale(). The easiest
solution is probably to call setlocale(LC_NUMERIC, "C") before
snprintf() (or at start-up ui/gtk.c), but opinions are welcome.
Berto
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-18 23:57 [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code Alberto Garcia
@ 2015-08-24 8:58 ` Daniel P. Berrange
2015-08-24 10:05 ` Markus Armbruster
1 sibling, 0 replies; 17+ messages in thread
From: Daniel P. Berrange @ 2015-08-24 8:58 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel
On Wed, Aug 19, 2015 at 01:57:55AM +0200, Alberto Garcia wrote:
> We have this code in qjson.c to produce JSON from a QFloat:
>
> QFloat *val = qobject_to_qfloat(obj);
> char buffer[1024];
> int len;
>
> len = snprintf(buffer, sizeof(buffer), "%f", qfloat_get_double(val));
> while (len > 0 && buffer[len - 1] == '0') {
> len--;
> }
>
>
> The problem here is that the output of snprintf() is locale-dependent,
> so depending on the locale we might get a ',' as decimal separator.
> That's not allowed by JSON and it actually breaks scripts/qmp/qmp.
>
> This seems to happen because of GTK+ calling setlocale(). The easiest
> solution is probably to call setlocale(LC_NUMERIC, "C") before
> snprintf() (or at start-up ui/gtk.c), but opinions are welcome.
I reckon calling setlocale at a key point in the startup process would
be a sensible thing todo.
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 :|
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-18 23:57 [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code Alberto Garcia
2015-08-24 8:58 ` Daniel P. Berrange
@ 2015-08-24 10:05 ` Markus Armbruster
2015-08-24 10:29 ` Alberto Garcia
1 sibling, 1 reply; 17+ messages in thread
From: Markus Armbruster @ 2015-08-24 10:05 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, Gerd Hoffmann
Copying maintainer.
Alberto Garcia <berto@igalia.com> writes:
> We have this code in qjson.c to produce JSON from a QFloat:
>
> QFloat *val = qobject_to_qfloat(obj);
> char buffer[1024];
> int len;
>
> len = snprintf(buffer, sizeof(buffer), "%f", qfloat_get_double(val));
> while (len > 0 && buffer[len - 1] == '0') {
> len--;
> }
>
>
> The problem here is that the output of snprintf() is locale-dependent,
> so depending on the locale we might get a ',' as decimal separator.
> That's not allowed by JSON and it actually breaks scripts/qmp/qmp.
>
> This seems to happen because of GTK+ calling setlocale(). The easiest
> solution is probably to call setlocale(LC_NUMERIC, "C") before
> snprintf() (or at start-up ui/gtk.c), but opinions are welcome.
A library calling setlocale() is a big no-no in my book.
Overriding LC_NUMERIC as you propose should fix this particular bug.
However, exposing unprepared code to locale is not a good idea for other
categories as well. LC_COLLATE and LC_CTYPE are even sneakier sources
of bugs in my experience. I'd really, really prefer to stay in the "C"
locale *completely*.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-24 10:05 ` Markus Armbruster
@ 2015-08-24 10:29 ` Alberto Garcia
2015-08-24 17:07 ` Markus Armbruster
0 siblings, 1 reply; 17+ messages in thread
From: Alberto Garcia @ 2015-08-24 10:29 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, Gerd Hoffmann
On Mon 24 Aug 2015 12:05:02 PM CEST, Markus Armbruster wrote:
>> This seems to happen because of GTK+ calling setlocale(). The easiest
>> solution is probably to call setlocale(LC_NUMERIC, "C") before
>> snprintf() (or at start-up ui/gtk.c), but opinions are welcome.
>
> A library calling setlocale() is a big no-no in my book.
>
> Overriding LC_NUMERIC as you propose should fix this particular bug.
>However, exposing unprepared code to locale is not a good idea for
>other categories as well. LC_COLLATE and LC_CTYPE are even sneakier
>sources of bugs in my experience. I'd really, really prefer to stay in
>the "C" locale *completely*.
You can prevent GTK+ from calling setlocale() by using
gtk_disable_setlocale() before gtk_init(), but note that setlocale() is
needed for gettext.
Berto
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-24 10:29 ` Alberto Garcia
@ 2015-08-24 17:07 ` Markus Armbruster
2015-08-24 17:18 ` Eric Blake
2015-08-24 17:56 ` Daniel P. Berrange
0 siblings, 2 replies; 17+ messages in thread
From: Markus Armbruster @ 2015-08-24 17:07 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, Gerd Hoffmann
Alberto Garcia <berto@igalia.com> writes:
> On Mon 24 Aug 2015 12:05:02 PM CEST, Markus Armbruster wrote:
>
>>> This seems to happen because of GTK+ calling setlocale(). The easiest
>>> solution is probably to call setlocale(LC_NUMERIC, "C") before
>>> snprintf() (or at start-up ui/gtk.c), but opinions are welcome.
>>
>> A library calling setlocale() is a big no-no in my book.
>>
>> Overriding LC_NUMERIC as you propose should fix this particular bug.
>>However, exposing unprepared code to locale is not a good idea for
>>other categories as well. LC_COLLATE and LC_CTYPE are even sneakier
>>sources of bugs in my experience. I'd really, really prefer to stay in
>>the "C" locale *completely*.
>
> You can prevent GTK+ from calling setlocale() by using
> gtk_disable_setlocale() before gtk_init(), but note that setlocale() is
> needed for gettext.
We can
(A) Internationalize our complete code base
(B) Run in the C locale
Breaks GTK's internationalization.
(C) Run in a mixed locale
Whenever something breaks, we switch another LC_ to the C locale.
Can partially break GTK's internationalization.
I happily concede that (A) would be best. Until the manpower to pull it
off appears, I recommend (B), because it's safer than (C), and avoids
inconsistent localization, such as German messages combined with
non-German number formatting.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-24 17:07 ` Markus Armbruster
@ 2015-08-24 17:18 ` Eric Blake
2015-08-25 7:54 ` Markus Armbruster
2015-08-24 17:56 ` Daniel P. Berrange
1 sibling, 1 reply; 17+ messages in thread
From: Eric Blake @ 2015-08-24 17:18 UTC (permalink / raw)
To: Markus Armbruster, Alberto Garcia; +Cc: qemu-devel, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]
On 08/24/2015 11:07 AM, Markus Armbruster wrote:
>> You can prevent GTK+ from calling setlocale() by using
>> gtk_disable_setlocale() before gtk_init(), but note that setlocale() is
>> needed for gettext.
>
> We can
>
> (A) Internationalize our complete code base
>
> (B) Run in the C locale
>
> Breaks GTK's internationalization.
>
> (C) Run in a mixed locale
>
> Whenever something breaks, we switch another LC_ to the C locale.
>
> Can partially break GTK's internationalization.
>
> I happily concede that (A) would be best. Until the manpower to pull it
> off appears, I recommend (B), because it's safer than (C), and avoids
> inconsistent localization, such as German messages combined with
> non-German number formatting.
Is it still possible for:
(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.
I don't know how well mingw supports thread-specific locales (POSIX
requires it, but we all know that mingw is behind the curve when it
comes to POSIX...); BSD and glibc should be okay, though.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-24 17:07 ` Markus Armbruster
2015-08-24 17:18 ` Eric Blake
@ 2015-08-24 17:56 ` Daniel P. Berrange
2015-08-24 20:27 ` Eric Blake
1 sibling, 1 reply; 17+ messages in thread
From: Daniel P. Berrange @ 2015-08-24 17:56 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Alberto Garcia, qemu-devel, Gerd Hoffmann
On Mon, Aug 24, 2015 at 07:07:38PM +0200, Markus Armbruster wrote:
> Alberto Garcia <berto@igalia.com> writes:
>
> > On Mon 24 Aug 2015 12:05:02 PM CEST, Markus Armbruster wrote:
> >
> >>> This seems to happen because of GTK+ calling setlocale(). The easiest
> >>> solution is probably to call setlocale(LC_NUMERIC, "C") before
> >>> snprintf() (or at start-up ui/gtk.c), but opinions are welcome.
> >>
> >> A library calling setlocale() is a big no-no in my book.
> >>
> >> Overriding LC_NUMERIC as you propose should fix this particular bug.
> >>However, exposing unprepared code to locale is not a good idea for
> >>other categories as well. LC_COLLATE and LC_CTYPE are even sneakier
> >>sources of bugs in my experience. I'd really, really prefer to stay in
> >>the "C" locale *completely*.
> >
> > You can prevent GTK+ from calling setlocale() by using
> > gtk_disable_setlocale() before gtk_init(), but note that setlocale() is
> > needed for gettext.
>
> We can
>
> (A) Internationalize our complete code base
>
> (B) Run in the C locale
>
> Breaks GTK's internationalization.
>
> (C) Run in a mixed locale
>
> Whenever something breaks, we switch another LC_ to the C locale.
>
> Can partially break GTK's internationalization.
>
> I happily concede that (A) would be best. Until the manpower to pull it
> off appears, I recommend (B), because it's safer than (C), and avoids
> inconsistent localization, such as German messages combined with
> non-German number formatting.
It seems the only thing that we really care about being localized is
the messages catalogue, so the GTK UI gets internationalization in
its menus / dialogs / etc. As such I think that we should do the
opposite of (C). ie run every LC_* in the C locale, except for
LC_MESSAGES which we allow to be localized.
This avoids any unpredictable functional consequences (like number
formatting) while still giving user decent localization in the UI
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 :|
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-24 17:56 ` Daniel P. Berrange
@ 2015-08-24 20:27 ` Eric Blake
2015-08-26 6:46 ` Gerd Hoffmann
0 siblings, 1 reply; 17+ messages in thread
From: Eric Blake @ 2015-08-24 20:27 UTC (permalink / raw)
To: Daniel P. Berrange, Markus Armbruster
Cc: Alberto Garcia, qemu-devel, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]
On 08/24/2015 11:56 AM, Daniel P. Berrange wrote:
>> (C) Run in a mixed locale
>>
>> Whenever something breaks, we switch another LC_ to the C locale.
>>
>> Can partially break GTK's internationalization.
>>
>> I happily concede that (A) would be best. Until the manpower to pull it
>> off appears, I recommend (B), because it's safer than (C), and avoids
>> inconsistent localization, such as German messages combined with
>> non-German number formatting.
>
> It seems the only thing that we really care about being localized is
> the messages catalogue, so the GTK UI gets internationalization in
> its menus / dialogs / etc. As such I think that we should do the
> opposite of (C). ie run every LC_* in the C locale, except for
> LC_MESSAGES which we allow to be localized.
>
> This avoids any unpredictable functional consequences (like number
> formatting) while still giving user decent localization in the UI
Except that the LC_MESSAGES catalog may include messages such as "blah
%d blah" that get translated for use as a printf argument, and the lack
of matching LC_NUMERIC will make the translation look wrong.
Translators often assume that their translation is being used with
everything else about the locale matching the locale of the translation.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-24 17:18 ` Eric Blake
@ 2015-08-25 7:54 ` Markus Armbruster
2015-08-25 8:15 ` Alberto Garcia
2015-08-26 9:13 ` Alberto Garcia
0 siblings, 2 replies; 17+ messages in thread
From: Markus Armbruster @ 2015-08-25 7:54 UTC (permalink / raw)
To: Eric Blake; +Cc: Alberto Garcia, qemu-devel, Gerd Hoffmann
Eric Blake <eblake@redhat.com> writes:
> On 08/24/2015 11:07 AM, Markus Armbruster wrote:
>
>>> You can prevent GTK+ from calling setlocale() by using
>>> gtk_disable_setlocale() before gtk_init(), but note that setlocale() is
>>> needed for gettext.
>>
>> We can
>>
>> (A) Internationalize our complete code base
>>
>> (B) Run in the C locale
>>
>> Breaks GTK's internationalization.
>>
>> (C) Run in a mixed locale
>>
>> Whenever something breaks, we switch another LC_ to the C locale.
>>
>> Can partially break GTK's internationalization.
>>
>> I happily concede that (A) would be best. Until the manpower to pull it
>> off appears, I recommend (B), because it's safer than (C), and avoids
>> inconsistent localization, such as German messages combined with
>> non-German number formatting.
>
> Is it still possible for:
>
> (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).
I'd feel much better about confining GTK in its own thread, and setting
only that thread's locale.
If that's impractical, perhaps we can switch locale just around GTK
code. Thread-locally, of course.
> I don't know how well mingw supports thread-specific locales (POSIX
> requires it, but we all know that mingw is behind the curve when it
> comes to POSIX...); BSD and glibc should be okay, though.
I have no idea. We could have configure checking whether we have
thread-local locale, and if not, disable internationalization entirely.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-25 7:54 ` Markus Armbruster
@ 2015-08-25 8:15 ` Alberto Garcia
2015-08-25 12:51 ` Markus Armbruster
2015-08-26 9:13 ` Alberto Garcia
1 sibling, 1 reply; 17+ messages in thread
From: Alberto Garcia @ 2015-08-25 8:15 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake; +Cc: qemu-devel, Gerd Hoffmann
On Tue 25 Aug 2015 09:54:42 AM CEST, Markus Armbruster wrote:
> 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).
>
> I'd feel much better about confining GTK in its own thread, and
> setting only that thread's locale.
FWIW GTK+ is not thread safe, all GTK+ code must run in the same thread,
so that should already be happening. I assume however that it's the same
thread that runs the monitor, so that might not be a solution in the
end.
Berto
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-25 8:15 ` Alberto Garcia
@ 2015-08-25 12:51 ` Markus Armbruster
0 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2015-08-25 12:51 UTC (permalink / raw)
To: Alberto Garcia; +Cc: qemu-devel, Gerd Hoffmann
Alberto Garcia <berto@igalia.com> writes:
> On Tue 25 Aug 2015 09:54:42 AM CEST, Markus Armbruster wrote:
>
>> 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).
>>
>> I'd feel much better about confining GTK in its own thread, and
>> setting only that thread's locale.
>
> FWIW GTK+ is not thread safe, all GTK+ code must run in the same thread,
> so that should already be happening. I assume however that it's the same
> thread that runs the monitor, so that might not be a solution in the
> end.
If we use thread-local locale for the GTK UI, then anything running in
that thread must either not depend on locale, or be properly
internationalized. Neither holds for the monitor (or pretty much any
non-trivial part of QEMU).
Can we give the GTK UI code its very own thread?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-24 20:27 ` Eric Blake
@ 2015-08-26 6:46 ` Gerd Hoffmann
2015-08-26 9:57 ` Daniel P. Berrange
0 siblings, 1 reply; 17+ messages in thread
From: Gerd Hoffmann @ 2015-08-26 6:46 UTC (permalink / raw)
To: Eric Blake; +Cc: Alberto Garcia, Markus Armbruster, qemu-devel
Hi,
> > It seems the only thing that we really care about being localized is
> > the messages catalogue, so the GTK UI gets internationalization in
> > its menus / dialogs / etc. As such I think that we should do the
> > opposite of (C). ie run every LC_* in the C locale, except for
> > LC_MESSAGES which we allow to be localized.
> >
> > This avoids any unpredictable functional consequences (like number
> > formatting) while still giving user decent localization in the UI
>
> Except that the LC_MESSAGES catalog may include messages such as "blah
> %d blah" that get translated for use as a printf argument, and the lack
> of matching LC_NUMERIC will make the translation look wrong.
> Translators often assume that their translation is being used with
> everything else about the locale matching the locale of the translation.
I don't think this is a big issue in practice. The menu item names are
pretty much the only thing translated in the qemu gtk ui ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-25 7:54 ` Markus Armbruster
2015-08-25 8:15 ` Alberto Garcia
@ 2015-08-26 9:13 ` Alberto Garcia
2015-08-26 9:22 ` Daniel P. Berrange
1 sibling, 1 reply; 17+ messages in thread
From: Alberto Garcia @ 2015-08-26 9:13 UTC (permalink / raw)
To: Markus Armbruster, Eric Blake; +Cc: qemu-devel, Gerd Hoffmann
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" ?
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.
Berto
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-26 9:13 ` Alberto Garcia
@ 2015-08-26 9:22 ` Daniel P. Berrange
2015-08-26 9:47 ` Markus Armbruster
0 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 9:22 UTC (permalink / raw)
To: Alberto Garcia; +Cc: Gerd Hoffmann, Markus Armbruster, qemu-devel
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 :|
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-26 9:22 ` Daniel P. Berrange
@ 2015-08-26 9:47 ` Markus Armbruster
0 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2015-08-26 9:47 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Alberto Garcia, Gerd Hoffmann, qemu-devel
"Daniel P. Berrange" <berrange@redhat.com> writes:
> 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:
[...]
For QEMU, I doubt sick string munging just to support GUI localization
on losing systems is worthwhile.
>> 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.
If we can't move GTK out of the main thread, we need to move everything
else out.
One more option: instead of switching back to the C locale around
identified troublemakers (and then chase them down one by one, bug by
bug), switch away from it just around the GUI. Thread-locally of
course. Callbacks may require extra care.
We should've stayed out of the GUI business.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-26 6:46 ` Gerd Hoffmann
@ 2015-08-26 9:57 ` Daniel P. Berrange
2015-08-26 12:04 ` Markus Armbruster
0 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrange @ 2015-08-26 9:57 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Alberto Garcia, Markus Armbruster, qemu-devel
On Wed, Aug 26, 2015 at 08:46:35AM +0200, Gerd Hoffmann wrote:
> Hi,
>
> > > It seems the only thing that we really care about being localized is
> > > the messages catalogue, so the GTK UI gets internationalization in
> > > its menus / dialogs / etc. As such I think that we should do the
> > > opposite of (C). ie run every LC_* in the C locale, except for
> > > LC_MESSAGES which we allow to be localized.
> > >
> > > This avoids any unpredictable functional consequences (like number
> > > formatting) while still giving user decent localization in the UI
> >
> > Except that the LC_MESSAGES catalog may include messages such as "blah
> > %d blah" that get translated for use as a printf argument, and the lack
> > of matching LC_NUMERIC will make the translation look wrong.
> > Translators often assume that their translation is being used with
> > everything else about the locale matching the locale of the translation.
>
> I don't think this is a big issue in practice. The menu item names are
> pretty much the only thing translated in the qemu gtk ui ...
Also we're talking about a tradeoff between functionally incorrect
formatting of JSON, vs "wrong looking" translations with no functional
impact, which probably won't even affect QEMU in reality.
In terms of minimizing hacks to QEMU, it seems like only localizing
LC_MESSAGES is a simple enough tradeoff to avoid functionally
incorrect behaviour with minimal downside and no code complexity
messing around with thread-locales, etc
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 :|
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code
2015-08-26 9:57 ` Daniel P. Berrange
@ 2015-08-26 12:04 ` Markus Armbruster
0 siblings, 0 replies; 17+ messages in thread
From: Markus Armbruster @ 2015-08-26 12:04 UTC (permalink / raw)
To: Daniel P. Berrange; +Cc: Alberto Garcia, Gerd Hoffmann, qemu-devel
"Daniel P. Berrange" <berrange@redhat.com> writes:
> On Wed, Aug 26, 2015 at 08:46:35AM +0200, Gerd Hoffmann wrote:
>> Hi,
>>
>> > > It seems the only thing that we really care about being localized is
>> > > the messages catalogue, so the GTK UI gets internationalization in
>> > > its menus / dialogs / etc. As such I think that we should do the
>> > > opposite of (C). ie run every LC_* in the C locale, except for
>> > > LC_MESSAGES which we allow to be localized.
>> > >
>> > > This avoids any unpredictable functional consequences (like number
>> > > formatting) while still giving user decent localization in the UI
>> >
>> > Except that the LC_MESSAGES catalog may include messages such as "blah
>> > %d blah" that get translated for use as a printf argument, and the lack
>> > of matching LC_NUMERIC will make the translation look wrong.
>> > Translators often assume that their translation is being used with
>> > everything else about the locale matching the locale of the translation.
>>
>> I don't think this is a big issue in practice. The menu item names are
>> pretty much the only thing translated in the qemu gtk ui ...
>
> Also we're talking about a tradeoff between functionally incorrect
> formatting of JSON, vs "wrong looking" translations with no functional
> impact, which probably won't even affect QEMU in reality.
>
> In terms of minimizing hacks to QEMU, it seems like only localizing
> LC_MESSAGES is a simple enough tradeoff to avoid functionally
> incorrect behaviour with minimal downside and no code complexity
> messing around with thread-locales, etc
It's a hack, and as such it needs a fat comment explaining it.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-08-26 12:04 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-18 23:57 [Qemu-devel] QEMU produces invalid JSON due to locale-dependent code Alberto Garcia
2015-08-24 8:58 ` Daniel P. Berrange
2015-08-24 10:05 ` Markus Armbruster
2015-08-24 10:29 ` Alberto Garcia
2015-08-24 17:07 ` Markus Armbruster
2015-08-24 17:18 ` Eric Blake
2015-08-25 7:54 ` Markus Armbruster
2015-08-25 8:15 ` Alberto Garcia
2015-08-25 12:51 ` Markus Armbruster
2015-08-26 9:13 ` Alberto Garcia
2015-08-26 9:22 ` Daniel P. Berrange
2015-08-26 9:47 ` Markus Armbruster
2015-08-24 17:56 ` Daniel P. Berrange
2015-08-24 20:27 ` Eric Blake
2015-08-26 6:46 ` Gerd Hoffmann
2015-08-26 9:57 ` Daniel P. Berrange
2015-08-26 12:04 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).