Fix breakage that had crept into setlocale() usage: once again we've

been bit by the fact that the locale functions return pointers to
modifiable variables.  I added some comments that might help us avoid
the mistake in future.

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 3b6114542beb5f3875dc39695f071e687ec8eb15..2ebea73036a7f3b0821ce7ce6397e4146b7f702f 100644 (file)
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -2,14 +2,14 @@
  *
  * PostgreSQL locale utilities
  *
- * $Header: /cvsroot/pgsql/src/backend/utils/adt/pg_locale.c,v 1.19 2002/09/04 20:31:28 momjian Exp $
- *
  * Portions Copyright (c) 2002, PostgreSQL Global Development Group
  *
+ * $Header: /cvsroot/pgsql/src/backend/utils/adt/pg_locale.c,v 1.20 2002/10/18 20:44:02 tgl Exp $
+ *
  *-----------------------------------------------------------------------
  */
 
-/*
+/*----------
  * Here is how the locale stuff is handled: LC_COLLATE and LC_CTYPE
  * are fixed by initdb, stored in pg_control, and cannot be changed.
  * Thus, the effects of strcoll(), strxfrm(), isupper(), toupper(),
@@ -20,14 +20,29 @@
  *
  * The other categories, LC_MONETARY, LC_NUMERIC, and LC_TIME are also
  * settable at run-time.  However, we don't actually set those locale
- * categories permanently.     This would have bizzare effects like no
+ * categories permanently.     This would have bizarre effects like no
  * longer accepting standard floating-point literals in some locales.
  * Instead, we only set the locales briefly when needed, cache the
  * required information obtained from localeconv(), and set them back.
- * The information is only used by the formatting functions (to_char,
- * etc.) and the money type.  For the user, this should all be
+ * The cached information is only used by the formatting functions
+ * (to_char, etc.) and the money type.  For the user, this should all be
  * transparent.  (Actually, LC_TIME doesn't do anything at all right
  * now.)
+ *
+ * !!! NOW HEAR THIS !!!
+ *
+ * We've been bitten repeatedly by this bug, so let's try to keep it in
+ * mind in future: on some platforms, the locale functions return pointers
+ * to static data that will be overwritten by any later locale function.
+ * Thus, for example, the obvious-looking sequence
+ *                     save = setlocale(category, NULL);
+ *                     if (!setlocale(category, value))
+ *                             fail = true;
+ *                     setlocale(category, save);
+ * DOES NOT WORK RELIABLY: on some platforms the second setlocale() call
+ * will change the memory save is pointing at.  To do this sort of thing
+ * safely, you *must* pstrdup what setlocale returns the first time.
+ *----------
  */
 
 
@@ -64,15 +79,19 @@ locale_xxx_assign(int category, const char *value, bool doit, bool interactive)
 
        save = setlocale(category, NULL);
        if (!save)
-               return NULL;
+               return NULL;                    /* won't happen, we hope */
+
+       /* save may be pointing at a modifiable scratch variable, see above */
+       save = pstrdup(save);
 
        if (!setlocale(category, value))
-               return NULL;
+               value = NULL;                   /* set failure return marker */
 
-       setlocale(category, save);
+       setlocale(category, save);      /* assume this won't fail */
+       pfree(save);
 
-       /* need to reload cache next time */
-       if (doit)
+       /* need to reload cache next time? */
+       if (doit && value != NULL)
                CurrentLocaleConvValid = false;
 
        return value;
@@ -99,7 +118,7 @@ locale_time_assign(const char *value, bool doit, bool interactive)
 
 
 /*
- * lc_messages takes effect immediately
+ * We allow LC_MESSAGES to actually be set globally.
  */
 const char *
 locale_messages_assign(const char *value, bool doit, bool interactive)
@@ -116,16 +135,7 @@ locale_messages_assign(const char *value, bool doit, bool interactive)
        }
        else
        {
-               char       *save;
-
-               save = setlocale(LC_MESSAGES, NULL);
-               if (!save)
-                       return NULL;
-
-               if (!setlocale(LC_MESSAGES, value))
-                       return NULL;
-
-               setlocale(LC_MESSAGES, save);
+               value = locale_xxx_assign(LC_MESSAGES, value, false, interactive);
        }
 #endif
        return value;
@@ -210,8 +220,13 @@ PGLC_localeconv(void)
 
        free_struct_lconv(&CurrentLocaleConv);
 
+       /* Set user's values of monetary and numeric locales */
        save_lc_monetary = setlocale(LC_MONETARY, NULL);
+       if (save_lc_monetary)
+               save_lc_monetary = pstrdup(save_lc_monetary);
        save_lc_numeric = setlocale(LC_NUMERIC, NULL);
+       if (save_lc_numeric)
+               save_lc_numeric = pstrdup(save_lc_numeric);
 
        setlocale(LC_MONETARY, locale_monetary);
        setlocale(LC_NUMERIC, locale_numeric);
@@ -221,7 +236,7 @@ PGLC_localeconv(void)
 
        /*
         * Must copy all values since restoring internal settings may
-        * overwrite
+        * overwrite localeconv()'s results.
         */
        CurrentLocaleConv = *extlconv;
        CurrentLocaleConv.currency_symbol = strdup(extlconv->currency_symbol);
@@ -236,8 +251,18 @@ PGLC_localeconv(void)
        CurrentLocaleConv.positive_sign = strdup(extlconv->positive_sign);
        CurrentLocaleConv.n_sign_posn = extlconv->n_sign_posn;
 
-       setlocale(LC_MONETARY, save_lc_monetary);
-       setlocale(LC_NUMERIC, save_lc_numeric);
+       /* Try to restore internal settings */
+       if (save_lc_monetary)
+       {
+               setlocale(LC_MONETARY, save_lc_monetary);
+               pfree(save_lc_monetary);
+       }
+
+       if (save_lc_numeric)
+       {
+               setlocale(LC_NUMERIC, save_lc_numeric);
+               pfree(save_lc_numeric);
+       }
 
        CurrentLocaleConvValid = true;
        return &CurrentLocaleConv;