Hi Karl,

Karl Williamson wrote on Mon, Mar 19, 2018 at 11:51:31AM -0600:

> I still believe that in my program the setlocale() returning C for 
> LC_ALL is a bug.  LC_CTYPE should have successfully been set to Romanian 
> UTF-8, and so LC_ALL isn't C.  Instead, it is a combination of C for all 
> the other categories, and UTF-8 for LC_CTYPE.  A return of just "C" 
> doesn't reflect that complexity.   There is a footnote in the ANSI/ISO 
> 9899-1990 C standard that the returned string must support that 
> heterogeneity, and that the return value be able to be used in a future 
> setlocale to get back to the original state.  Your setlocale violates 
> the standard therefore, and harms your portability goal.

Thanks again for reporting this bug, i fixed it with the commit below.

> (And to be 
> consistent, LC_ALL should have been the heterogenous composition of all 
> the locale strings that the program sets, even if they just boil down to 
> C or UTF-8.  Programs that run on other OS's are expecting this, and 
> again your portability goal is compromised)

I fear i still don't understand what you mean here.
Is that part also fixed with my commit?
If not, or if you are not sure (because you lack a test system),
can you state as precisely as possibly which call, that currently
has which effect and returns which value should have which other
effect and return which other value instead?

> I urge you to update your man page.  If it had set out what you've 
> stated in your replies, it would have saved our project a bunch of hours 
> of work.

Sorry for that.  We do regard bugs in manuals as about as important
as bugs in the implementation, but just as with code, it can't be
helped that sometimes, bugs and omissions happen anyway.

Yours,
  Ingo


CVSROOT:        /cvs
Module name:    src
Changes by:     schwa...@cvs.openbsd.org        2018/03/29 10:34:25

Modified files:
        lib/libc/locale: setlocale.c 
        regress/lib/libc/locale/setlocale: setlocale.c 

Log message:
Fix three bugs in setlocale(3):
1. setlocale(LC_ALL, "A"); setlocale(LC_CTYPE, "T"); setlocale(LC_ALL, NULL);
must return "A/T/A/A/A/A", not "A".  Fix this by always initializing the
LC_ALL entry of newgl to "" in dupgl().  Reported by Karl Williamson
<public at khwilliamson dot com> on bugs@, thanks!
2. Do not leak newgl when strdup(3) fails in setlocale(3).
3. For setlocale(LC_ALL, "C/C/fr_FR.UTF-8/C/C/C"); correctly set
_GlobalRuneLocale; i found 2. and 3. while looking at the code.
Feedback on a buggy earlier version and OK martijn@.


Index: lib/libc/locale/setlocale.c
===================================================================
RCS file: /cvs/src/lib/libc/locale/setlocale.c,v
retrieving revision 1.27
diff -u -p -r1.27 setlocale.c
--- lib/libc/locale/setlocale.c 5 Sep 2017 03:16:13 -0000       1.27
+++ lib/libc/locale/setlocale.c 29 Mar 2018 16:32:53 -0000
@@ -42,8 +42,8 @@ dupgl(char **oldgl)
        if ((newgl = calloc(_LC_LAST, sizeof(*newgl))) == NULL) 
                return NULL;
        for (ic = LC_ALL; ic < _LC_LAST; ic++) {
-               if ((newgl[ic] = strdup(oldgl != NULL ?
-                   oldgl[ic] : ic == LC_ALL ? "" : "C")) == NULL) {
+               if ((newgl[ic] = strdup(ic == LC_ALL ? "" :
+                   oldgl == NULL ? "C" : oldgl[ic])) == NULL) {
                        freegl(newgl);
                        return NULL;
                }
@@ -92,8 +92,10 @@ setlocale(int category, const char *locn
                if (category == LC_ALL && strchr(locname, '/') != NULL) {
 
                        /* One value for each category. */
-                       if ((firstname = strdup(locname)) == NULL)
+                       if ((firstname = strdup(locname)) == NULL) {
+                               freegl(newgl);
                                return NULL;
+                       }
                        nextname = firstname;
                        for (ic = 1; ic < _LC_LAST; ic++)
                                if (nextname == NULL || changegl(ic,
@@ -184,7 +186,7 @@ done:
                        global_locale = newgl;
                        if (category == LC_ALL || category == LC_CTYPE)
                                _GlobalRuneLocale =
-                                   strchr(global_locname, '.') == NULL ?
+                                   strchr(newgl[LC_CTYPE], '.') == NULL ?
                                    &_DefaultRuneLocale : _Utf8RuneLocale;
                } else {
                        freegl(newgl);
Index: regress/lib/libc/locale/setlocale/setlocale.c
===================================================================
RCS file: /cvs/src/regress/lib/libc/locale/setlocale/setlocale.c,v
retrieving revision 1.3
diff -u -p -r1.3 setlocale.c
--- regress/lib/libc/locale/setlocale/setlocale.c       25 Feb 2017 07:28:32 
-0000      1.3
+++ regress/lib/libc/locale/setlocale/setlocale.c       29 Mar 2018 16:32:53 
-0000
@@ -75,7 +75,6 @@ main(int argc, char *argv[])
 
        /* load from env */
        /* NOTE: we don't support non-C locales for some categories */
-       /*test_setlocale("fr_FR.UTF-8", LC_ALL, "");*/ /* set */
        test_setlocale("fr_FR.UTF-8", LC_CTYPE, ""); /* set */
        test_setlocale("fr_FR.UTF-8", LC_MESSAGES, ""); /* set */
        test_MB_CUR_MAX(4);
@@ -113,6 +112,7 @@ main(int argc, char *argv[])
        test_setlocale("C", LC_ALL, "C"); /* reset */
        test_setlocale("invalid.UTF-8", LC_CTYPE, "invalid.UTF-8"); /* set */
        test_setlocale("invalid.UTF-8", LC_CTYPE, NULL);
+       test_setlocale("C/invalid.UTF-8/C/C/C/C", LC_ALL, NULL);
        test_MB_CUR_MAX(4);
 
        /* with invalid codeset (is an error) */

Reply via email to