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

Christian Stimming stimming at tuhh.de
Sat Jan 29 15:11:08 EST 2011


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.

Regards,

Christian


More information about the gnucash-devel mailing list