r20189 - gnucash/trunk/src/backend/dbi - Handle localization-ignorance bug in libdbi by setting LC_NUMERIC locale

[John Ralls]

On Jan 29, 2011, at 12:11 PM, Christian Stimming wrote:

> Am Samstag, 29. Januar 2011 schrieb John Ralls:
>>>> I believe unfortunately it doesn't work this easy way: The returned
>>>> value of setlocale( ) might point to static storage, which will
>>>> subsequently be overwritten by setlocale("C"), so you won't be setting
>>>> it to the original value by the third setlocale. Instead, unfortunately
>>>> you will have to g_strdup the returned value of setlocale()
>>>> immediately, then change the locale, then do the broken libdbi stuff,
>>>> then setlocale() back to the original value, then g_free the original
>>>> value.
>>> 
>>> So I guess you should refactor the common code parts into some function
>>> in, say, core-utils or similar...
>> 
>> I see three ways to refactor:
>> a) Have a static variable at file scope (because it has to be accessed by
>> two functions,  switch_locale() and restore_locale()... or generalized to
>> push_locale() and pop_locale(). (C++ could use a singleton locale class
>> with the locale-stack as a member variable... but then C++ already
>> gracefully handles multiple locales.) b) Define a function type
>> TempLocaleFunc that could be passed in to a locale-switching function and
>> coerce all of the  activities that need to be in a different locale to fit
>> the function description. c) Define macros SWITCH_LOCALE( save, new ) and
>> RESTORE_LOCALE( save ), where "save" is the char* to park the old locale
>> in and "new" is the new locale.
>> 
>> They all seem like overkill to avoid duplicating 4 lines of code.
> 
> I was thinking along the lines of requiring a local gchar* variable that is 
> returned by the first function call and passed again into the second function 
> call. Also, I don't ask for refactoring because of the amount of code but 
> rather because this issue is obviously tricky to get right, and your example 
> here just showed again that a "trivial-looking implementation" does not get it 
> right.
> 
> With the requirement of a local gchar* variable, I thought the dbi-backend 
> code can look like this:
> 
> gchar *tmplocale = gnc_setlocale_switch("C");
> // ... Do the locale-sensitive libdbi calls here
> gnc_setlocale_restore(tmplocale);
> // The tmplocale pointer is free'd now
> 
> This at least gets the tricky parts of the setlocale() calls out of our 
> application code and unifies them into one place. I surely admit this isn't 
> much of a gain in terms of lines of code, but as I said, I the benefit of a 
> common pair of functions here is to get it right in one place.

Heh. When I looked at gnc-ui-utils.c I found that it was an implementation of (a) with the stack, and that it's been there since at least 2001. So I cleaned it up a bit and used it in gnc-backend-dbi.c. (r20192)

Regards,
John Ralls