* [setlocale]: return only one copy if all six parts of locale are same
@ 2017-03-18 7:37 He X
2017-03-18 12:33 ` Rich Felker
0 siblings, 1 reply; 6+ messages in thread
From: He X @ 2017-03-18 7:37 UTC (permalink / raw)
To: musl
[-- Attachment #1.1: Type: text/plain, Size: 39 bytes --]
As i suggest on IRC, here's the patch.
[-- Attachment #1.2: Type: text/html, Size: 64 bytes --]
[-- Attachment #2: setlocale.diff --]
[-- Type: text/plain, Size: 1037 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,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;
}
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [setlocale]: return only one copy if all six parts of locale are same 2017-03-18 7:37 [setlocale]: return only one copy if all six parts of locale are same He X @ 2017-03-18 12:33 ` Rich Felker 2017-03-18 13:36 ` He X 0 siblings, 1 reply; 6+ messages in thread From: Rich Felker @ 2017-03-18 12:33 UTC (permalink / raw) To: musl 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [setlocale]: return only one copy if all six parts of locale are same 2017-03-18 12:33 ` Rich Felker @ 2017-03-18 13:36 ` He X 2017-03-18 13:48 ` He X 0 siblings, 1 reply; 6+ messages in thread From: He X @ 2017-03-18 13:36 UTC (permalink / raw) To: musl [-- 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; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [setlocale]: return only one copy if all six parts of locale are same 2017-03-18 13:36 ` He X @ 2017-03-18 13:48 ` He X 2017-03-18 14:39 ` Rich Felker 0 siblings, 1 reply; 6+ messages in thread From: He X @ 2017-03-18 13:48 UTC (permalink / raw) To: musl [-- Attachment #1.1: Type: text/plain, Size: 2782 bytes --] oops, i missed UNLOCK(lock). 2017-03-18 21:36 GMT+08:00 He X <xw897002528@gmail.com>: > 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: 5041 bytes --] [-- Attachment #2: setlocale.patch --] [-- Type: text/x-patch, Size: 980 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,31 @@ } } 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) { + UNLOCK(lock); + 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; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [setlocale]: return only one copy if all six parts of locale are same 2017-03-18 13:48 ` He X @ 2017-03-18 14:39 ` Rich Felker 2017-03-18 15:00 ` He X 0 siblings, 1 reply; 6+ messages in thread From: Rich Felker @ 2017-03-18 14:39 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 561 bytes --] On Sat, Mar 18, 2017 at 09:48:41PM +0800, He X wrote: > oops, i missed UNLOCK(lock). > > [...] > >> > >> 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 > >> > > > > By simpler I meant something like the attached. Rich [-- Attachment #2: setlocale.diff --] [-- Type: text/plain, Size: 774 bytes --] diff --git a/src/locale/setlocale.c b/src/locale/setlocale.c index 8dae5a4..ca1646f 100644 --- a/src/locale/setlocale.c +++ b/src/locale/setlocale.c @@ -48,10 +48,13 @@ char *setlocale(int cat, const char *name) } } char *s = buf; + const char *part; + int same = 0; for (i=0; i<LC_ALL; i++) { const struct __locale_map *lm = libc.global_locale.cat[i]; - const char *part = lm ? lm->name : "C"; + if (lm == libc.global_locale.cat[0]) same++; + part = lm ? lm->name : "C"; size_t l = strlen(part); memcpy(s, part, l); s[l] = ';'; @@ -59,7 +62,7 @@ char *setlocale(int cat, const char *name) } *--s = 0; UNLOCK(lock); - return buf; + return same==LC_ALL ? part : buf; } char *ret = setlocale_one_unlocked(cat, name); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [setlocale]: return only one copy if all six parts of locale are same 2017-03-18 14:39 ` Rich Felker @ 2017-03-18 15:00 ` He X 0 siblings, 0 replies; 6+ messages in thread From: He X @ 2017-03-18 15:00 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 784 bytes --] Wow, tested. Thx for improvements. Learnt a lots from recent patches related to locale. Sincerely thank you, rich :) xhe 2017-03-18 22:39 GMT+08:00 Rich Felker <dalias@libc.org>: > On Sat, Mar 18, 2017 at 09:48:41PM +0800, He X wrote: > > oops, i missed UNLOCK(lock). > > > > [...] > > >> > > >> 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 > > >> > > > > > > > > By simpler I meant something like the attached. > > Rich > [-- Attachment #2: Type: text/html, Size: 1528 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-18 15:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-03-18 7:37 [setlocale]: return only one copy if all six parts of locale are same He X 2017-03-18 12:33 ` Rich Felker 2017-03-18 13:36 ` He X 2017-03-18 13:48 ` He X 2017-03-18 14:39 ` Rich Felker 2017-03-18 15:00 ` He X
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).