Re: [PATCH] Read locale settings from environment

[Corinna Vinschen <vinschen at redhat dot com>]

On Feb 19 16:57, Jeff Johnston wrote:
> Hi Corinna,
>
>  There is a problem with this patch.  The code path you have made changes 
> to
> applies when locale is set to "C" or "".  In the case of "C" the old code 
> should still be in place (i.e. if !strcmp(locale, "C")).  A check is needed 
> for when !strcmp(locale, "").  If you make that fix, it should be fine.
>
>  Another problem exists with the current code.  The return value from 
> LC_ALL should be a concatenation of the various locale settings separated 
> by a special character (e.g. ':').  The LC_ALL category needs to check if 
> that is the form of the input string given and separate them out and call 
> for each category.  This way, the original settings can be restored on a 
> subsequent call to setlocale() with the string given back from LC_ALL.  
> This form only applies to LC_ALL and is not valid input for any other 
> category.

There's more broken in setlocale.  For instance, if locale is "C" or
"", the variable locale_name is set to "C".  But afterwards, the tests
are still using locale instead of locale_name.  And worse, locale[1]
is tested, even though locale could be "" at this point.  It also
just occured to me that the current code disallows *any* other setting
of LC_ALL except for "C" or "".  I'll rework the function a bit.  Stay
tuned.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat