This is the mail archive of the libc-hacker@sources.redhat.com mailing list for the glibc project.

Note that libc-hacker is a closed list. You may look at the archives of this list, but subscription and posting are not open.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

[PATCH] setlocale changes


Hi!

My recent changes to setlocale broke IBM JDK 1.1.8.
While I think IBM locale is buggy, I'm not 100% sure and so I expect other
programs would not work either.
The issue is when is pointer returned by setlocale still valid:

     The string returned by setlocale() is such that a subsequent call with that string and its associated category will
     restore that part of the program's locale. The string returned must not be modified by the program, but may be
     overwritten by a subsequent call to setlocale().

I think glibc complies to this, but does not if the second sentence would
include by a subsqeuent call to setlocale() with the specified category.

IBM JDK does basically:

char *p = setlocale(LC_CTYPE, "");
setlocale(LC_ALL, p);
/* use p - as use p you could consider e.g. setlocale(LC_CTYPE, p); */

The following patch tries to help these programs, by making sure that as
long as some locale name is the name of at least one category, the pointer
is valid (previously we freed it and allocated the same string again in the
above example), so it is valid at least until the next setlocale call with
the same category (with LC_ALL being the only exception, where any setlocale
makes that string invalid - if it would be desirable to ensure that as well,
as in:
char *p = setlocale(LC_ALL, "");
setlocale(LC_CTYPE, "C");
setlocale(LC_ALL, p);
then I can add that easily too
).

2001-01-02  Jakub Jelinek  <jakub@redhat.com>

	* locale/setlocale.c (INC_REFCNT, DEC_REFCNT): Define.
	(dupname, freename): New functions.
	(new_composite_name, setname, setlocale): Use them.  Make sure
	as long as some locale name is set to at least one category,
	setlocale returns the same pointer.

--- libc/locale/setlocale.c.jj	Mon Dec  4 15:07:35 2000
+++ libc/locale/setlocale.c	Tue Jan  2 18:07:29 2001
@@ -1,4 +1,4 @@
-/* Copyright (C) 1991, 92, 95-99, 2000 Free Software Foundation, Inc.
+/* Copyright (C) 1991, 92, 95-99, 2000, 2001 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -16,6 +16,7 @@
    write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    Boston, MA 02111-1307, USA.  */
 
+#include <assert.h>
 #include <alloca.h>
 #include <argz.h>
 #include <errno.h>
@@ -131,9 +132,45 @@ extern int _nl_msg_cat_cntr;
     return NULL;							      \
   } while (0)
 
+#define INC_REFCNT(name) (++(((unsigned char *)(name))[-1]))
+#define DEC_REFCNT(name) (--(((unsigned char *)(name))[-1]))
+
+/* Duplicate a string unless it is already present
+   in _nl_current_names array.  */
+static inline const char *
+dupname (const char *name)
+{
+  int i;
+  size_t len;
+  char *ret;
+
+  for (i = 0; i < __LC_LAST; ++i)
+    if (! strcmp (_nl_current_names[i], name))
+      {
+	INC_REFCNT(_nl_current_names[i]);
+	return _nl_current_names[i];
+      }
+  len = strlen (name);
+  ret = malloc (len + 2);
+  if (ret)
+    {
+      ret[0] = 1;
+      memcpy (++ret, name, len + 1);
+    }
+  return ret;
+}
+
+
+/* Free a category name.  */
+static inline void
+freename (char *name)
+{
+  if (name != _nl_C_name && ! DEC_REFCNT (name))
+    free (name - 1);
+}
 
 /* Construct a new composite name.  */
-static inline char *
+static inline const char *
 new_composite_name (int category, const char *newnames[__LC_LAST])
 {
   size_t last_len = 0;
@@ -157,18 +194,16 @@ new_composite_name (int category, const 
   if (same)
     {
       /* All the categories use the same name.  */
-      if (strcmp (newnames[0], _nl_C_name) == 0
-	  || strcmp (newnames[0], _nl_POSIX_name) == 0)
-	return (char *) _nl_C_name;
+      if (newnames[0] != _nl_C_name)
+	INC_REFCNT (newnames[0]);
 
-      new = malloc (last_len + 1);
-
-      return new == NULL ? NULL : memcpy (new, newnames[0], last_len + 1);
+      return newnames[0];
     }
 
-  new = malloc (cumlen);
+  new = malloc (cumlen + 1);
   if (new == NULL)
     return NULL;
+  *new++ = 1;
   p = new;
   for (i = 0; i < __LC_LAST; ++i)
     if (i != LC_ALL)
@@ -183,6 +218,12 @@ new_composite_name (int category, const 
 	*p++ = ';';
       }
   p[-1] = '\0';		/* Clobber the last ';'.  */
+  if (!strcmp (new, _nl_current_names[LC_ALL]))
+    {
+      INC_REFCNT (_nl_current_names[LC_ALL]);
+      free (new - 1);
+      return _nl_current_names[LC_ALL];
+    }
   return new;
 }
 
@@ -191,12 +232,7 @@ new_composite_name (int category, const 
 static inline void
 setname (int category, const char *name)
 {
-  if (_nl_current_names[category] == name)
-    return;
-
-  if (_nl_current_names[category] != _nl_C_name)
-    free ((char *) _nl_current_names[category]);
-
+  freename ((char *) _nl_current_names[category]);
   _nl_current_names[category] = name;
 }
 
@@ -214,13 +250,17 @@ setdata (int category, struct locale_dat
 }
 
 
+
 char *
 setlocale (int category, const char *locale)
 {
   char *locale_path;
   size_t locale_path_len;
   const char *locpath_var;
-  char *composite;
+  const char *composite;
+
+  /* Make sure we don't wrap around in reference counting.  */
+  assert (__LC_LAST * 2 == (unsigned char)(__LC_LAST * 2));
 
   /* Sanity check for CATEGORY argument.  */
   if (__builtin_expect (category, 0) < 0
@@ -326,7 +366,17 @@ setlocale (int category, const char *loc
 	    /* Make a copy of locale name.  */
 	    if (newnames[category] != _nl_C_name)
 	      {
-		newnames[category] = strdup (newnames[category]);
+		int i;
+
+		for (i = category + 1; i < __LC_LAST; ++i)
+		  if (i != LC_ALL && !strcmp (newnames[i], newnames[category]))
+		    {
+		      INC_REFCNT (newnames[i]);
+		      newnames[category] = newnames[i];
+		      break;
+		    }
+		if (i == __LC_LAST)
+		  newnames[category] = dupname (newnames[category]);
 		if (newnames[category] == NULL)
 		  break;
 	      }
@@ -352,16 +402,16 @@ setlocale (int category, const char *loc
 	}
       else
 	for (++category; category < __LC_LAST; ++category)
-	  if (category != LC_ALL && newnames[category] != _nl_C_name)
-	    free ((char *) newnames[category]);
+	  if (category != LC_ALL)
+	    freename ((char *) newnames[category]);
 
       /* Critical section left.  */
       __libc_lock_unlock (__libc_setlocale_lock);
 
-      /* Free the resources (the locale path variable.  */
+      /* Free the resources (the locale path variable).  */
       free (locale_path);
 
-      return composite;
+      return (char *) composite;
     }
   else
     {
@@ -391,7 +441,7 @@ setlocale (int category, const char *loc
       /* Make a copy of locale name.  */
       if (newname[0] != _nl_C_name)
 	{
-	  newname[0] = strdup (newname[0]);
+	  newname[0] = dupname (newname[0]);
 	  if (newname[0] == NULL)
 	    goto abort_single;
 	}
@@ -400,8 +450,7 @@ setlocale (int category, const char *loc
       composite = new_composite_name (category, newname);
       if (composite == NULL)
 	{
-	  if (newname[0] != _nl_C_name)
-	    free ((char *) newname[0]);
+	  freename ((char *) newname[0]);
 
 	  /* Say that we don't have any data loaded.  */
 	abort_single:


	Jakub

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]