mailing list of musl libc
 help / color / mirror / code / Atom feed
* [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).