Re: [xine-devel] problematic use of setlocale in xinelib

[Sander J. <s.j...@gm...>]

On Fri, Dec 4, 2009 at 10:11 AM, Andreas Mohr <an...@li...> wrote:
> Hi,
>
> [manual, private reply]
>
>> I did a grep on setlocale in the xine source code and I saw a few problematic ones:
>>
>> In utils.c:563
>>
>> char *xine_get_system_encoding(void) {
>>   char *codeset = NULL;
>>
>> #ifdef HAVE_NL_LANGINFO
>>   setlocale(LC_ALL, "");
>>   codeset = nl_langinfo(CODESET);
>> #endif
>
> Is that really all that xinelib does? Too careless, I'd say.
>
> wxWidgets (2.8.10) does:
>
> #if defined(HAVE_LANGINFO_H) && defined(CODESET)
>    // GNU libc provides current character set this way (this conforms
>    // to Unix98)
>    char *oldLocale = strdup(setlocale(LC_CTYPE, NULL));
>    setlocale(LC_CTYPE, "");
>    const char *alang = nl_langinfo(CODESET);
>    setlocale(LC_CTYPE, oldLocale);
>    free(oldLocale);
>
> IOW, it at least changes back the locale to the original setting once
> the query is done.

Well, if this was a single threaded application, I would agree, but
since xine is heavely multithreaded, I don't think we shouldn't even
bother. Either solution is wrong. I have been developing a patch that
would query the locale on xine_init only, and then reuse it instead.
That would almost fix the whole xine-library, except for the dxr3
video output driver, which needs a "C" string to float conversion.

>
> Of course it then doesn't help though that wx does this setlocale() call
> for _every_ resource lookup it does (my app has a couple thousands),
> causing lots of potential trouble for other threads.
>
> So it would probably be a relief if you could send an updated xine patch
> which restores the locale to original values.
>
>
> Oh my, what a huge mess that terrible thread disease brought upon us...
> (bloating runtime libraries to no end to try to achieve thread-safety
> and thus severely penalizing proper per-process-only applications,
> and that all simply because a certain sorry operating system is thoroughly
> process-challenged)
>

I think the ultimate solution would be making use of xlocale.h which
allows for per-thread and per-function locale objects. I know it is
supported on modern linux and modern windows (a quick scan of msdn
indicated those functions are there). I'm not sure if xine-lib would
want to take that route, since it may support some older platforms
that may not have this.

At least, the xlocale is a solution for apps linking to libraries that
happen to change the locale from under them.

Cheers,

Sander




-- 
"The sands of time were eroded by
The river of constant change."