[illumos-Developer] Problems in setlocale and codereview

[Joerg Schilling]

"Garrett D'Amore" <garrett at nexenta.com> wrote:

> Thanks.  Can you post a bug on this (if not already done)?
>
> I had used the order of the array from Solaris, but didn't realize that
> the implementation was sensitive to the ordering.  So my bad.

Here is my patch:

--- setlocale.c.orig	Di Aug 31 20:49:26 2010
+++ setlocale.c	Do Sep  2 17:33:11 2010
@@ -125,6 +125,8 @@
 	if (!*locale) {
 		if (category == LC_ALL) {
 			for (i = 0; i < NUM_CATS; ++i) {
+				if (i == LC_ALL)
+					continue;
 				env = __get_locale_env(i);
 				if (strlen(env) > ENCODING_LEN) {
 					errno = EINVAL;
@@ -152,8 +154,11 @@
 				errno = EINVAL;
 				return (NULL);
 			}
-			for (i = 1; i < NUM_CATS; ++i)
+			for (i = 0; i < NUM_CATS; ++i) {
+				if (i == LC_ALL)
+					continue;
 				(void) strcpy(new_categories[i], locale);
+			}
 		} else {
 			char	*buf;
 			char	*save;
@@ -160,6 +165,14 @@
 
 			buf = alloca(strlen(locale) + 1);
 
+			/*
+			 * XXX the for loop starts with buf being uninitialized.
+			 * XXX it may work after calling strcpy() but what is
+			 * XXX the correct source r?? locale??
+			 * XXX LC_ALL is incorrect here, use NUM_CATS instead
+			 * XXX and check in a way that make the code independent
+			 * XXX from the order in categories[]
+			 */
 			for (i = 0, save = NULL; i <= LC_ALL; i++) {
 				r = strtok_r(buf, "/", &save);
 				if (r == NULL) {
@@ -192,11 +205,15 @@
 	if (category != LC_ALL)
 		return (loadlocale(category));
 
-	for (i = 0; i < LC_ALL; ++i) {
+	for (i = 0; i < NUM_CATS; ++i) {
+		if (i == LC_ALL)
+			continue;
 		(void) strcpy(saved_categories[i], current_categories[i]);
 		if (loadlocale(i) == NULL) {
 			saverr = errno;
-			for (j = 1; j < i; j++) {
+			for (j = 0; j < i; j++) {
+				if (i == LC_ALL)
+					continue;
 				(void) strcpy(new_categories[j],
 				    saved_categories[j]);
 				if (loadlocale(j) == NULL) {
@@ -211,6 +228,9 @@
 	return (currentlocale());
 }
 
+/*
+ * XXX is it really correct to exclude LC_COLLATE?
+ */
 static char *
 currentlocale(void)
 {
@@ -218,9 +238,17 @@
 
 	(void) strcpy(current_locale_string, current_categories[0]);
 
-	for (i = 1; i < LC_ALL; ++i)
+	for (i = 0; i < NUM_CATS; ++i)
+		if (i == LC_ALL)
+			continue;
+		if (i == LC_COLLATE)
+			continue;
 		if (strcmp(current_categories[1], current_categories[i])) {
-			for (i = 1; i < LC_ALL; ++i) {
+			for (i = 0; i < NUM_CATS; ++i) {
+				if (i == LC_ALL)
+					continue;
+				if (i == LC_COLLATE)
+					continue;
 				(void) strcat(current_locale_string, "/");
 				(void) strcat(current_locale_string,
 				    current_categories[i]);
@@ -285,7 +313,7 @@
 	const char *env;
 
 	/* 1. check LC_ALL. */
-	env = getenv(categories[0]);
+	env = getenv(categories[LC_ALL]);
 
 	/* 2. check LC_* */
 	if (env == NULL || !*env)


Jörg

-- 
 EMail:joerg at schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin
       js at cs.tu-berlin.de                (uni)  
       joerg.schilling at fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/
 URL:  http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily