Re: setlocale: make calls with NULL argument multithread-safe

[Bruno Haible]

Part of this new code in setlocale.c would also be useful for localename.c.
I'm therefore moving it to module 'setlocale-null'. At the same time, I'm
renaming the function setlocale_null to setlocale_null_r (in analogy with
getlogin_r, ttyname_r, ptsname_r).


2019-12-18  Bruno Haible  <address@hidden>

        setlocale-null: Make API more useful.
        * lib/locale.in.h (setlocale_null_r): Renamed from setlocale_null. All
        callers changed.
        (setlocale_null): New declaration.
        * lib/setlocale_null.c (setlocale_null_androidfix): New function,
        extracted from setlocale_null_unlocked.
        (setlocale_null_unlocked): Invoke it.
        (setlocale_null_r): Renamed from setlocale_null.
        (setlocale_null): New function, extracted from setlocale_mtsafe in
        setlocale.c.
        * lib/setlocale.c: Don't include <errno.h>.
        (setlocale_mtsafe): Invoke setlocale_null.
        * lib/setlocale-lock.c: Update comments.
        * doc/posix-functions/setlocale.texi: Mention both functions.

diff --git a/lib/locale.in.h b/lib/locale.in.h
index 67e6020..f4eccc2 100644
--- a/lib/locale.in.h
+++ b/lib/locale.in.h
@@ -220,7 +220,7 @@ _GL_WARN_ON_USE (setlocale, "setlocale works differently on 
native Windows - "
    In native Windows, there are 5 categories, and the maximum total length is
    55+5*58.  */
 # define SETLOCALE_NULL_ALL_MAX (148+12*256+1)
-/* setlocale_null (CATEGORY, BUF, BUFSIZE) is like setlocale (CATEGORY, NULL),
+/* setlocale_null_r (CATEGORY, BUF, BUFSIZE) is like setlocale (CATEGORY, 
NULL),
    except that
      - it is guaranteed to be multithread-safe,
      - it returns the resulting locale category name or locale name in the
@@ -234,14 +234,25 @@ _GL_WARN_ON_USE (setlocale, "setlocale works differently 
on native Windows - "
    result is returned in BUF, but still NUL-terminated if BUFSIZE > 0.
    For this call to be multithread-safe, *all* calls to
    setlocale (CATEGORY, NULL) in all other threads must have been converted
-   to use setlocale_null as well, and the other threads must not make other
-   setlocale invocations (since changing the global locale has side effects
-   on all threads).  */
-_GL_FUNCDECL_SYS (setlocale_null, int,
+   to use setlocale_null_r or setlocale_null as well, and the other threads
+   must not make other setlocale invocations (since changing the global locale
+   has side effects on all threads).  */
+_GL_FUNCDECL_SYS (setlocale_null_r, int,
                   (int category, char *buf, size_t bufsize)
                   _GL_ARG_NONNULL ((2)));
-_GL_CXXALIAS_SYS (setlocale_null, int,
+_GL_CXXALIAS_SYS (setlocale_null_r, int,
                   (int category, char *buf, size_t bufsize));
+_GL_CXXALIASWARN (setlocale_null_r);
+/* setlocale_null (CATEGORY) is like setlocale (CATEGORY, NULL), except that
+   it is guaranteed to be multithread-safe.
+   The return value is NULL if CATEGORY is invalid.
+   For this call to be multithread-safe, *all* calls to
+   setlocale (CATEGORY, NULL) in all other threads must have been converted
+   to use setlocale_null_r or setlocale_null as well, and the other threads
+   must not make other setlocale invocations (since changing the global locale
+   has side effects on all threads).  */
+_GL_FUNCDECL_SYS (setlocale_null, const char *, (int category));
+_GL_CXXALIAS_SYS (setlocale_null, const char *, (int category));
 _GL_CXXALIASWARN (setlocale_null);
 #endif
 
diff --git a/lib/setlocale_null.c b/lib/setlocale_null.c
index 8072a45..c201257 100644
--- a/lib/setlocale_null.c
+++ b/lib/setlocale_null.c
@@ -55,6 +55,37 @@
 /* Use the system's setlocale() function, not the gnulib override, here.  */
 #undef setlocale
 
+static const char *
+setlocale_null_androidfix (int category)
+{
+  const char *result = setlocale (category, NULL);
+
+#ifdef __ANDROID__
+  if (result == NULL)
+    switch (category)
+      {
+      case LC_CTYPE:
+      case LC_NUMERIC:
+      case LC_TIME:
+      case LC_COLLATE:
+      case LC_MONETARY:
+      case LC_MESSAGES:
+      case LC_ALL:
+      case LC_PAPER:
+      case LC_NAME:
+      case LC_ADDRESS:
+      case LC_TELEPHONE:
+      case LC_MEASUREMENT:
+        result = "C";
+        break;
+      default:
+        break;
+      }
+#endif
+
+  return result;
+}
+
 static int
 setlocale_null_unlocked (int category, char *buf, size_t bufsize)
 {
@@ -105,30 +136,7 @@ setlocale_null_unlocked (int category, char *buf, size_t 
bufsize)
         }
     }
 #else
-  const char *result = setlocale (category, NULL);
-
-# ifdef __ANDROID__
-  if (result == NULL)
-    switch (category)
-      {
-      case LC_CTYPE:
-      case LC_NUMERIC:
-      case LC_TIME:
-      case LC_COLLATE:
-      case LC_MONETARY:
-      case LC_MESSAGES:
-      case LC_ALL:
-      case LC_PAPER:
-      case LC_NAME:
-      case LC_ADDRESS:
-      case LC_TELEPHONE:
-      case LC_MEASUREMENT:
-        result = "C";
-        break;
-      default:
-        break;
-      }
-# endif
+  const char *result = setlocale_null_androidfix (category);
 
   if (result == NULL)
     {
@@ -257,7 +265,7 @@ setlocale_null_with_lock (int category, char *buf, size_t 
bufsize)
 #endif
 
 int
-setlocale_null (int category, char *buf, size_t bufsize)
+setlocale_null_r (int category, char *buf, size_t bufsize)
 {
 #if SETLOCALE_NULL_ALL_MTSAFE
 # if SETLOCALE_NULL_ONE_MTSAFE
@@ -287,3 +295,116 @@ setlocale_null (int category, char *buf, size_t bufsize)
 # endif
 #endif
 }
+
+const char *
+setlocale_null (int category)
+{
+#if SETLOCALE_NULL_ALL_MTSAFE && SETLOCALE_NULL_ONE_MTSAFE
+  return setlocale_null_androidfix (category);
+#else
+
+  /* This call must be multithread-safe.  To achieve this without using
+     thread-local storage:
+       1. We use a specific static buffer for each possible CATEGORY
+          argument.  So that different threads can call setlocale_mtsafe
+          with different CATEGORY arguments, without interfering.
+       2. We use a simple strcpy or memcpy to fill this static buffer.
+          Filling it through, for example, strcpy + strcat would not be
+          guaranteed to leave the buffer's contents intact if another thread
+          is currently accessing it.  If necessary, the contents is first
+          assembled in a stack-allocated buffer.  */
+  if (category == LC_ALL)
+    {
+# if SETLOCALE_NULL_ALL_MTSAFE
+      return setlocale_null_androidfix (LC_ALL);
+# else
+      char buf[SETLOCALE_NULL_ALL_MAX];
+      static char resultbuf[SETLOCALE_NULL_ALL_MAX];
+
+      if (setlocale_null_r (LC_ALL, buf, sizeof (buf)))
+        return "C";
+      strcpy (resultbuf, buf);
+      return resultbuf;
+# endif
+    }
+  else
+    {
+# if SETLOCALE_NULL_ONE_MTSAFE
+      return setlocale_null_androidfix (category);
+# else
+      enum
+        {
+          LC_CTYPE_INDEX,
+          LC_NUMERIC_INDEX,
+          LC_TIME_INDEX,
+          LC_COLLATE_INDEX,
+          LC_MONETARY_INDEX,
+          LC_MESSAGES_INDEX,
+#  ifdef LC_PAPER
+          LC_PAPER_INDEX,
+#  endif
+#  ifdef LC_NAME
+          LC_NAME_INDEX,
+#  endif
+#  ifdef LC_ADDRESS
+          LC_ADDRESS_INDEX,
+#  endif
+#  ifdef LC_TELEPHONE
+          LC_TELEPHONE_INDEX,
+#  endif
+#  ifdef LC_MEASUREMENT
+          LC_MEASUREMENT_INDEX,
+#  endif
+#  ifdef LC_IDENTIFICATION
+          LC_IDENTIFICATION_INDEX,
+#  endif
+          LC_INDICES_COUNT
+        }
+        i;
+      char buf[SETLOCALE_NULL_MAX];
+      static char resultbuf[LC_INDICES_COUNT][SETLOCALE_NULL_MAX];
+      int err;
+
+      err = setlocale_null_r (category, buf, sizeof (buf));
+      if (err == EINVAL)
+        return NULL;
+      if (err)
+        return "C";
+
+      switch (category)
+        {
+        case LC_CTYPE:          i = LC_CTYPE_INDEX;          break;
+        case LC_NUMERIC:        i = LC_NUMERIC_INDEX;        break;
+        case LC_TIME:           i = LC_TIME_INDEX;           break;
+        case LC_COLLATE:        i = LC_COLLATE_INDEX;        break;
+        case LC_MONETARY:       i = LC_MONETARY_INDEX;       break;
+        case LC_MESSAGES:       i = LC_MESSAGES_INDEX;       break;
+#  ifdef LC_PAPER
+        case LC_PAPER:          i = LC_PAPER_INDEX;          break;
+#  endif
+#  ifdef LC_NAME
+        case LC_NAME:           i = LC_NAME_INDEX;           break;
+#  endif
+#  ifdef LC_ADDRESS
+        case LC_ADDRESS:        i = LC_ADDRESS_INDEX;        break;
+#  endif
+#  ifdef LC_TELEPHONE
+        case LC_TELEPHONE:      i = LC_TELEPHONE_INDEX;      break;
+#  endif
+#  ifdef LC_MEASUREMENT
+        case LC_MEASUREMENT:    i = LC_MEASUREMENT_INDEX;    break;
+#  endif
+#  ifdef LC_IDENTIFICATION
+        case LC_IDENTIFICATION: i = LC_IDENTIFICATION_INDEX; break;
+#  endif
+        default:
+          /* If you get here, a #ifdef LC_xxx is missing.  */
+          abort ();
+        }
+
+      strcpy (resultbuf[i], buf);
+      return resultbuf[i];
+# endif
+    }
+#endif
+}
diff --git a/lib/setlocale.c b/lib/setlocale.c
index da28a1a..55b8fa4 100644
--- a/lib/setlocale.c
+++ b/lib/setlocale.c
@@ -29,7 +29,6 @@
 /* Specification.  */
 #include <locale.h>
 
-#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -71,111 +70,7 @@ char *
 setlocale_mtsafe (int category, const char *locale)
 {
   if (locale == NULL)
-    {
-      /* This call must be multithread-safe.  To achieve this without using
-         thread-local storage:
-           1. We use a specific static buffer for each possible CATEGORY
-              argument.  So that different threads can call setlocale_mtsafe
-              with different CATEGORY arguments, without interfering.
-           2. We use a simple strcpy or memcpy to fill this static buffer.
-              Filling it through, for example, strcpy + strcat would not be
-              guaranteed to leave the buffer's contents intact if another 
thread
-              is currently accessing it.  If necessary, the contents is first
-              assembled in a stack-allocated buffer.  */
-      if (category == LC_ALL)
-        {
-#  if SETLOCALE_NULL_ALL_MTSAFE
-          return setlocale (LC_ALL, NULL);
-#  else
-          char buf[SETLOCALE_NULL_ALL_MAX];
-          static char resultbuf[SETLOCALE_NULL_ALL_MAX];
-
-          if (setlocale_null (LC_ALL, buf, sizeof (buf)))
-            return (char *) "C";
-          strcpy (resultbuf, buf);
-          return resultbuf;
-#  endif
-        }
-      else
-        {
-#  if SETLOCALE_NULL_ONE_MTSAFE
-          return setlocale (category, NULL);
-#  else
-          enum
-            {
-              LC_CTYPE_INDEX,
-              LC_NUMERIC_INDEX,
-              LC_TIME_INDEX,
-              LC_COLLATE_INDEX,
-              LC_MONETARY_INDEX,
-              LC_MESSAGES_INDEX,
-#   ifdef LC_PAPER
-              LC_PAPER_INDEX,
-#   endif
-#   ifdef LC_NAME
-              LC_NAME_INDEX,
-#   endif
-#   ifdef LC_ADDRESS
-              LC_ADDRESS_INDEX,
-#   endif
-#   ifdef LC_TELEPHONE
-              LC_TELEPHONE_INDEX,
-#   endif
-#   ifdef LC_MEASUREMENT
-              LC_MEASUREMENT_INDEX,
-#   endif
-#   ifdef LC_IDENTIFICATION
-              LC_IDENTIFICATION_INDEX,
-#   endif
-              LC_INDICES_COUNT
-            }
-            i;
-          char buf[SETLOCALE_NULL_MAX];
-          static char resultbuf[LC_INDICES_COUNT][SETLOCALE_NULL_MAX];
-          int err;
-
-          err = setlocale_null (category, buf, sizeof (buf));
-          if (err == EINVAL)
-            return NULL;
-          if (err)
-            return (char *) "C";
-
-          switch (category)
-            {
-            case LC_CTYPE:          i = LC_CTYPE_INDEX;          break;
-            case LC_NUMERIC:        i = LC_NUMERIC_INDEX;        break;
-            case LC_TIME:           i = LC_TIME_INDEX;           break;
-            case LC_COLLATE:        i = LC_COLLATE_INDEX;        break;
-            case LC_MONETARY:       i = LC_MONETARY_INDEX;       break;
-            case LC_MESSAGES:       i = LC_MESSAGES_INDEX;       break;
-#   ifdef LC_PAPER
-            case LC_PAPER:          i = LC_PAPER_INDEX;          break;
-#   endif
-#   ifdef LC_NAME
-            case LC_NAME:           i = LC_NAME_INDEX;           break;
-#   endif
-#   ifdef LC_ADDRESS
-            case LC_ADDRESS:        i = LC_ADDRESS_INDEX;        break;
-#   endif
-#   ifdef LC_TELEPHONE
-            case LC_TELEPHONE:      i = LC_TELEPHONE_INDEX;      break;
-#   endif
-#   ifdef LC_MEASUREMENT
-            case LC_MEASUREMENT:    i = LC_MEASUREMENT_INDEX;    break;
-#   endif
-#   ifdef LC_IDENTIFICATION
-            case LC_IDENTIFICATION: i = LC_IDENTIFICATION_INDEX; break;
-#   endif
-            default:
-              /* If you get here, a #ifdef LC_xxx is missing.  */
-              abort ();
-            }
-
-          strcpy (resultbuf[i], buf);
-          return resultbuf[i];
-#  endif
-        }
-    }
+    return (char *) setlocale_null (category);
   else
     return setlocale (category, locale);
 }
diff --git a/lib/duplocale.c b/lib/duplocale.c
index d22a447..da94d44 100644
--- a/lib/duplocale.c
+++ b/lib/duplocale.c
@@ -72,7 +72,7 @@ rpl_duplocale (locale_t locale)
       locale_t base_copy;
       unsigned int i;
 
-      err = setlocale_null (LC_CTYPE, base_name, sizeof (base_name));
+      err = setlocale_null_r (LC_CTYPE, base_name, sizeof (base_name));
       if (err)
         {
           errno = err;
@@ -88,7 +88,7 @@ rpl_duplocale (locale_t locale)
           int category_mask = categories[i].mask;
           char name[SETLOCALE_NULL_MAX];
 
-          err = setlocale_null (category, name, sizeof (name));
+          err = setlocale_null_r (category, name, sizeof (name));
           if (err)
             {
               errno = err;
diff --git a/lib/hard-locale.c b/lib/hard-locale.c
index 4a2adab..5eaa7b6 100644
--- a/lib/hard-locale.c
+++ b/lib/hard-locale.c
@@ -28,7 +28,7 @@ hard_locale (int category)
 {
   char locale[SETLOCALE_NULL_MAX];
 
-  if (setlocale_null (category, locale, sizeof (locale)))
+  if (setlocale_null_r (category, locale, sizeof (locale)))
     return false;
 
   return !(strcmp (locale, "C") == 0 || strcmp (locale, "POSIX") == 0);
diff --git a/lib/nl_langinfo.c b/lib/nl_langinfo.c
index ff0e936..2c16511 100644
--- a/lib/nl_langinfo.c
+++ b/lib/nl_langinfo.c
@@ -51,7 +51,7 @@ ctype_codeset (void)
   char *codeset;
   size_t codesetlen;
 
-  if (setlocale_null (LC_CTYPE, locale, sizeof (locale)))
+  if (setlocale_null_r (LC_CTYPE, locale, sizeof (locale)))
     locale[0] = '\0';
 
   codeset = buf;
diff --git a/lib/setlocale-lock.c b/lib/setlocale-lock.c
index ed6ab9f..9c47afc 100644
--- a/lib/setlocale-lock.c
+++ b/lib/setlocale-lock.c
@@ -1,4 +1,4 @@
-/* Return the internal lock used by setlocale_null.
+/* Return the internal lock used by setlocale_null_r.
    Copyright (C) 2019 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
@@ -18,7 +18,7 @@
 
 #include <config.h>
 
-/* This file defines the internal lock used by setlocale_null.
+/* This file defines the internal lock used by setlocale_null_r.
    It is a separate compilation unit, so that only one copy of it is
    present when linking statically.  */
 
@@ -41,7 +41,7 @@ __declspec(dllexport) CRITICAL_SECTION 
*gl_get_setlocale_null_lock (void);
 static glwthread_initguard_t guard = GLWTHREAD_INITGUARD_INIT;
 static CRITICAL_SECTION lock;
 
-/* Returns the internal lock used by setlocale_null.  */
+/* Returns the internal lock used by setlocale_null_r.  */
 CRITICAL_SECTION *
 gl_get_setlocale_null_lock (void)
 {
@@ -77,7 +77,7 @@ static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 __declspec(dllexport) pthread_mutex_t *gl_get_setlocale_null_lock (void);
 # endif
 
-/* Returns the internal lock used by setlocale_null.  */
+/* Returns the internal lock used by setlocale_null_r.  */
 pthread_mutex_t *
 gl_get_setlocale_null_lock (void)
 {
@@ -101,7 +101,7 @@ atomic_init (void)
   init_needed = 0;
 }
 
-/* Returns the internal lock used by setlocale_null.  */
+/* Returns the internal lock used by setlocale_null_r.  */
 mtx_t *
 gl_get_setlocale_null_lock (void)
 {
diff --git a/tests/test-setlocale_null-all.c b/tests/test-setlocale_null-all.c
index 45eb27e..abf2df4 100644
--- a/tests/test-setlocale_null-all.c
+++ b/tests/test-setlocale_null-all.c
@@ -1,4 +1,4 @@
-/* Multithread-safety test for setlocale_null (LC_ALL, ...).
+/* Multithread-safety test for setlocale_null_r (LC_ALL, ...).
    Copyright (C) 2019 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
@@ -66,7 +66,7 @@ thread1_func (void *arg)
     {
       char buf[SETLOCALE_NULL_ALL_MAX];
 
-      if (setlocale_null (LC_ALL, buf, sizeof (buf)))
+      if (setlocale_null_r (LC_ALL, buf, sizeof (buf)))
         abort ();
       if (strcmp (expected, buf) != 0)
         {
@@ -86,8 +86,8 @@ thread2_func (void *arg)
     {
       char buf[SETLOCALE_NULL_ALL_MAX];
 
-      setlocale_null (LC_NUMERIC, buf, sizeof (buf));
-      setlocale_null (LC_ALL, buf, sizeof (buf));
+      setlocale_null_r (LC_NUMERIC, buf, sizeof (buf));
+      setlocale_null_r (LC_ALL, buf, sizeof (buf));
     }
 
   /*NOTREACHED*/
diff --git a/tests/test-setlocale_null-one.c b/tests/test-setlocale_null-one.c
index 9e9bc12..a83e485 100644
--- a/tests/test-setlocale_null-one.c
+++ b/tests/test-setlocale_null-one.c
@@ -1,4 +1,4 @@
-/* Multithread-safety test for setlocale_null (LC_xxx, ...).
+/* Multithread-safety test for setlocale_null_r (LC_xxx, ...).
    Copyright (C) 2019 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
@@ -66,7 +66,7 @@ thread1_func (void *arg)
     {
       char buf[SETLOCALE_NULL_MAX];
 
-      if (setlocale_null (LC_NUMERIC, buf, sizeof (buf)))
+      if (setlocale_null_r (LC_NUMERIC, buf, sizeof (buf)))
         abort ();
       if (strcmp (expected, buf) != 0)
         {
@@ -86,8 +86,8 @@ thread2_func (void *arg)
     {
       char buf[SETLOCALE_NULL_MAX];
 
-      setlocale_null (LC_NUMERIC, buf, sizeof (buf));
-      setlocale_null (LC_TIME, buf, sizeof (buf));
+      setlocale_null_r (LC_NUMERIC, buf, sizeof (buf));
+      setlocale_null_r (LC_TIME, buf, sizeof (buf));
     }
 
   /*NOTREACHED*/
diff --git a/tests/test-setlocale_null.c b/tests/test-setlocale_null.c
index 003cfc9..4244804 100644
--- a/tests/test-setlocale_null.c
+++ b/tests/test-setlocale_null.c
@@ -1,4 +1,4 @@
-/* Test of setlocale_null function.
+/* Test of setlocale_null_r function.
    Copyright (C) 2019 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
@@ -27,6 +27,6 @@ static char buf[SETLOCALE_NULL_ALL_MAX];
 int
 main ()
 {
-  /* Check that setlocale_null () can be used with $(LIB_SETLOCALE_NULL).  */
-  return setlocale_null (LC_ALL, buf, sizeof (buf)) != 0;
+  /* Check that setlocale_null_r() can be used with $(LIB_SETLOCALE_NULL).  */
+  return setlocale_null_r (LC_ALL, buf, sizeof (buf)) != 0;
 }
diff --git a/doc/posix-functions/setlocale.texi 
b/doc/posix-functions/setlocale.texi
index d8282b6..11364d3 100644
--- a/doc/posix-functions/setlocale.texi
+++ b/doc/posix-functions/setlocale.texi
@@ -34,7 +34,7 @@ platforms:
 musl libc, macOS, FreeBSD, NetBSD, OpenBSD, AIX, Haiku, Cygwin.
 To make these invocations multithread-safe, you need the Gnulib module
 @code{setlocale}, or you need to change the code to invoke 
@code{setlocale_null}
-instead.
+or @code{setlocale_null_r} instead.
 @end itemize
 
 Portability problems not fixed by Gnulib: