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

[John Ralls]

On Jan 29, 2011, at 1:48 AM, Christian Stimming wrote:

> Am Samstag, 29. Januar 2011 schrieb Christian Stimming:
>>> Trac: http://svn.gnucash.org/trac/changeset/20189
>>> 
>>> Modified:
>>>   gnucash/trunk/src/backend/dbi/gnc-backend-dbi.c
>> 
>> 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.
>> 
>> Seems like this is asking for some refactoring into separate functions...
> 
> Oh, and the very same thing is already done in two other gnucash files:
> 
> src/app-utils/gnc-ui-util.c:            const char *thislocale = 
> setlocale(LC_ALL, NULL);
> src/app-utils/gnc-ui-util.c:    saved_locale = g_strdup (setlocale (LC_ALL, 
> NULL));
> src/app-utils/gnc-ui-util.c:    setlocale (LC_ALL, locale);
> src/app-utils/gnc-ui-util.c:    setlocale (LC_ALL, saved_locale);
> src/gnome/druid-hierarchy.c:    locale = g_strdup(setlocale(LC_MESSAGES, 
> NULL));
> src/gnome/druid-hierarchy.c:    /* On win32, setlocale() doesn't say anything 
> useful. Use
> src/gnome/druid-hierarchy.c:     * setlocale can sometimes return NULL instead 
> of "C"
> src/gnome/druid-hierarchy.c:    locale = g_strdup(setlocale(LC_ALL, NULL) ?
> src/gnome/druid-hierarchy.c:                      setlocale(LC_ALL, NULL) : 
> "C");
> 
> 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. 

But thanks for pointing out the error... I'll fix it this morning, and I'll make sure that the other two instances are correct.

Regards,
John Ralls