Re: [setlocale]: return only one copy if all six parts of locale are same

[He X <xw897002528@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2605 bytes --]

tested:
[xhe@xhe-PC locale]$ ./a.out
C
zh_CN.UTF-8
zh_CN.UTF-8
[xhe@xhe-PC locale]$ cat test.c
#include <locale.h>
int main() {
printf("%s\n", setlocale(LC_ALL, NULL));
printf("%s\n", setlocale(LC_ALL, ""));
printf("%s\n", setlocale(LC_ALL, NULL));
}

2017-03-18 20:33 GMT+08:00 Rich Felker <dalias@libc.org>:

> On Sat, Mar 18, 2017 at 07:37:57AM +0000, He X wrote:
> > As i suggest on IRC, here's the patch.
>
> > --- musl-1.1.16/src/locale/setlocale.c        2017-03-17
> 17:49:15.767952411 +0000
> > +++ musl-1.1.16/src/locale/setlocale.c        2017-03-17
> 17:49:15.767952411 +0000
> > @@ -48,16 +48,33 @@
> >                       }
> >               }
> >               char *s = buf;
> > -             for (i=0; i<LC_ALL; i++) {
> > +             const struct __locale_map *flm =
> > +                     libc.global_locale.cat[0];
> > +             const char *fpart = flm ? flm->name : "C";
> > +             size_t l = strlen(fpart);
> > +             memcpy(s, fpart, l);
> > +             s[l] = 0;
> > +             i=1;
> > +             do {
> >                       const struct __locale_map *lm =
> >                               libc.global_locale.cat[i];
> >                       const char *part = lm ? lm->name : "C";
> > -                     size_t l = strlen(part);
> > -                     memcpy(s, part, l);
> > -                     s[l] = ';';
> > -                     s += l+1;
> > +                     if (strcmp(s, part)) break;
> > +                     i++;
> > +             } while (i<LC_ALL);
> > +
> > +             if (i != LC_ALL) {
> > +                     for (i=0; i<LC_ALL; i++) {
> > +                             const struct __locale_map *lm =
> > +                                     libc.global_locale.cat[i];
> > +                             const char *part = lm ? lm->name : "C";
> > +                             size_t l = strlen(part);
> > +                             memcpy(s, part, l);
> > +                             s[l] = ';';
> > +                             s += l+1;
> > +                     }
> > +                     *--s = 0;
> >               }
> > -             *--s = 0;
> >               UNLOCK(lock);
> >               return buf;
> >       }
>
> I think the results of this patch can be achieved much more simply.
> Just compare lm pointers (rather than string contents) for equality at
> each iteration in the current loop, counting how many were equal to
> libc.global_locale.cat[0]. If that number equals LC_ALL, return
> libc.global_locale.cat[0]->name (or "C" if the lm is null) rather than
> buf.
>
> Rich
>

[-- Attachment #1.2: Type: text/html, Size: 4515 bytes --]

[-- Attachment #2: setlocale.patch --]
[-- Type: text/x-patch, Size: 961 bytes --]

--- musl-1.1.16/src/locale/setlocale.c	2017-03-17 17:49:15.767952411 +0000
+++ musl-1.1.16/src/locale/setlocale.c	2017-03-17 17:49:15.767952411 +0000
@@ -48,16 +48,30 @@
 			}
 		}
 		char *s = buf;
-		for (i=0; i<LC_ALL; i++) {
+ 		const struct __locale_map *flm =
+ 			libc.global_locale.cat[0];
+ 		i=1;
+ 		do {
 			const struct __locale_map *lm =
 				libc.global_locale.cat[i];
-			const char *part = lm ? lm->name : "C";
-			size_t l = strlen(part);
-			memcpy(s, part, l);
-			s[l] = ';';
-			s += l+1;
+ 			if (lm != flm) break;
+ 			i++;
+ 		} while (i<LC_ALL);
+ 
+ 		if (i == LC_ALL) {
+			return flm ? flm->name : "C";
+		} else {
+ 			for (i=0; i<LC_ALL; i++) {
+ 				const struct __locale_map *lm =
+ 					libc.global_locale.cat[i];
+ 				const char *part = lm ? lm->name : "C";
+ 				size_t l = strlen(part);
+ 				memcpy(s, part, l);
+ 				s[l] = ';';
+ 				s += l+1;
+ 			}
+ 			*--s = 0;
 		}
-		*--s = 0;
 		UNLOCK(lock);
 		return buf;
 	}