Make PGLC_setlocale() static, and document that it can't be used safely

for any other purpose than PGLC_localeconv()'s internal save/restore of
locale settings.  Fix cash.c to call PGLC_localeconv() rather than
making a direct call to localeconv() --- the old way, if PGLC_localeconv()
had already cached a locale result, it would be overwritten by the first
cash_in or cash_out operation, leading to wrong-locale results later.
Probably no demonstrable bug today, since we only appear to be looking
at the LC_MONETARY results which should be the same anyway, but definitely
a gotcha waiting to strike.

Author: tglsfdc

src/backend/utils/adt/cash.c

@@ -9,18 +9,23 @@
  * workings can be found in the book "Software Solutions in C" by
  * Dale Schumacher, Academic Press, ISBN: 0-12-632360-7.
  *
- * $Header: /cvsroot/pgsql/src/backend/utils/adt/cash.c,v 1.47 2000/11/25 20:33:52 tgl Exp $
+ * $Header: /cvsroot/pgsql/src/backend/utils/adt/cash.c,v 1.48 2000/11/25 22:43:08 tgl Exp $
  */
 
+#include "postgres.h"
+
 #include <limits.h>
 #include <ctype.h>
 #include <math.h>
+#ifdef USE_LOCALE
 #include <locale.h>
+#endif
 
-#include "postgres.h"
 #include "miscadmin.h"
 #include "utils/builtins.h"
 #include "utils/cash.h"
+#include "utils/pg_locale.h"
+
 
 static const char *num_word(Cash value);
 
@@ -31,11 +36,6 @@ static const char *num_word(Cash value);
 #define LAST_PAREN		(TERMINATOR - 1)
 #define LAST_DIGIT		(LAST_PAREN - 1)
 
-#ifdef USE_LOCALE
-static struct lconv *lconvert = NULL;
-
-#endif
-
 
 /*
  * Cash is a pass-by-ref SQL type, so we must pass and return pointers.
@@ -82,11 +82,11 @@ cash_in(PG_FUNCTION_ARGS)
 				ssymbol,
 				psymbol,
 			   *nsymbol;
-
 #ifdef USE_LOCALE
-	if (lconvert == NULL)
-		lconvert = localeconv();
+	struct lconv *lconvert = PGLC_localeconv();
+#endif
 
+#ifdef USE_LOCALE
 	/*
 	 * frac_digits will be CHAR_MAX in some locales, notably C.  However,
 	 * just testing for == CHAR_MAX is risky, because of compilers like
@@ -238,11 +238,11 @@ cash_out(PG_FUNCTION_ARGS)
 				dsymbol,
 			   *nsymbol;
 	char		convention;
-
 #ifdef USE_LOCALE
-	if (lconvert == NULL)
-		lconvert = localeconv();
+	struct lconv *lconvert = PGLC_localeconv();
+#endif
 
+#ifdef USE_LOCALE
 	/* see comments about frac_digits in cash_in() */
 	points = lconvert->frac_digits;
 	if (points < 0 || points > 10)

src/backend/utils/adt/pg_locale.c

@@ -1,21 +1,18 @@
-
 /* -----------------------------------------------------------------------
  * pg_locale.c
  *
- * $Header: /cvsroot/pgsql/src/backend/utils/adt/pg_locale.c,v 1.6 2000/08/29 04:41:47 momjian Exp $
+ *	 The PostgreSQL locale utils.
  *
  *
- *	 Portions Copyright (c) 1999-2000, PostgreSQL, Inc
+ * $Header: /cvsroot/pgsql/src/backend/utils/adt/pg_locale.c,v 1.7 2000/11/25 22:43:08 tgl Exp $
  *
- *	 The PostgreSQL locale utils.
+ *	 Portions Copyright (c) 1999-2000, PostgreSQL, Inc
  *
  *	Karel Zak - Zakkr
  *
  * -----------------------------------------------------------------------
  */
 
-#include <stdio.h>
-
 #include "postgres.h"
 
 #ifdef USE_LOCALE
@@ -28,11 +25,13 @@
 
 static struct lconv *CurrentLocaleConv = NULL;
 
+static void PGLC_setlocale(PG_LocaleCategories * lc);
+
 /*------
- * Return in PG_LocaleCategories current locale setting
+ * Return in PG_LocaleCategories the current locale settings
  *------
  */
-PG_LocaleCategories *
+void
 PGLC_current(PG_LocaleCategories * lc)
 {
 	lc->lang = getenv("LANG");
@@ -45,7 +44,6 @@ PGLC_current(PG_LocaleCategories * lc)
 #ifdef LC_MESSAGES
 	lc->lc_messages = setlocale(LC_MESSAGES, NULL);
 #endif
-	return lc;
 }
 
 
@@ -55,7 +53,7 @@ PGLC_current(PG_LocaleCategories * lc)
  * Print a PG_LocaleCategories struct as DEBUG
  *------
  */
-PG_LocaleCategories *
+static void
 PGLC_debug_lc(PG_LocaleCategories * lc)
 {
 #ifdef LC_MESSAGES
@@ -73,72 +71,86 @@ PGLC_debug_lc(PG_LocaleCategories * lc)
 		 , lc->lc_messages
 #endif
 	);
-
-	return lc;
 }
 
 #endif
 
 /*------
  * Set locales via a PG_LocaleCategories struct
+ *
+ * NB: it would be very dangerous to set the locale values to any random
+ * choice of locale, since that could cause indexes to become corrupt, etc.
+ * Therefore this routine is NOT exported from this module.  It should be
+ * used only to restore previous locale settings during PGLC_localeconv.
  *------
  */
-PG_LocaleCategories *
+static void
 PGLC_setlocale(PG_LocaleCategories * lc)
 {
+	if (!setlocale(LC_COLLATE, lc->lc_collate))
+		elog(NOTICE, "pg_setlocale(): 'LC_COLLATE=%s' cannot be honored.",
+			 lc->lc_collate);
+
 	if (!setlocale(LC_CTYPE, lc->lc_ctype))
-		elog(NOTICE, "pg_setlocale(): 'LC_CTYPE=%s' cannot be honored.", lc->lc_ctype);
+		elog(NOTICE, "pg_setlocale(): 'LC_CTYPE=%s' cannot be honored.",
+			 lc->lc_ctype);
 
 	if (!setlocale(LC_NUMERIC, lc->lc_numeric))
-		elog(NOTICE, "pg_setlocale(): 'LC_NUMERIC=%s' cannot be honored.", lc->lc_numeric);
+		elog(NOTICE, "pg_setlocale(): 'LC_NUMERIC=%s' cannot be honored.",
+			 lc->lc_numeric);
 
 	if (!setlocale(LC_TIME, lc->lc_time))
-		elog(NOTICE, "pg_setlocale(): 'LC_TIME=%s' cannot be honored.", lc->lc_time);
-
-	if (!setlocale(LC_COLLATE, lc->lc_collate))
-		elog(NOTICE, "pg_setlocale(): 'LC_COLLATE=%s' cannot be honored.", lc->lc_collate);
+		elog(NOTICE, "pg_setlocale(): 'LC_TIME=%s' cannot be honored.",
+			 lc->lc_time);
 
 	if (!setlocale(LC_MONETARY, lc->lc_monetary))
-		elog(NOTICE, "pg_setlocale(): 'LC_MONETARY=%s' cannot be honored.", lc->lc_monetary);
+		elog(NOTICE, "pg_setlocale(): 'LC_MONETARY=%s' cannot be honored.",
+			 lc->lc_monetary);
+
 #ifdef LC_MESSAGES
 	if (!setlocale(LC_MESSAGES, lc->lc_messages))
-		elog(NOTICE, "pg_setlocale(): 'LC_MESSAGE=%s' cannot be honored.", lc->lc_messages);
+		elog(NOTICE, "pg_setlocale(): 'LC_MESSAGE=%s' cannot be honored.",
+			 lc->lc_messages);
 #endif
-	return lc;
 }
 
 /*------
  * Return the POSIX lconv struct (contains number/money formatting information)
- * with locale information for *all* categories.
- * => Returned lconv is *independent* on current locale catogories setting - in
- * contrast to standard localeconv().
+ * with locale information for all categories.  Note that returned lconv
+ * does not depend on currently active category settings, but on external
+ * environment variables for locale.
+ *
+ * XXX we assume that restoring old category settings via setlocale() will
+ * not immediately corrupt the static data returned by localeconv().
+ * How portable is this?
  *
- * ! libc prepare memory space for lconv itself and all returned strings in
- *	 lconv are *static strings*.
+ * XXX in any case, there certainly must not be any other calls to
+ * localeconv() anywhere in the backend, else the values reported here
+ * will be overwritten with the Postgres-internal locale settings.
  *------
  */
 struct lconv *
 PGLC_localeconv(void)
 {
 	PG_LocaleCategories lc;
-	
+
+	/* Did we do it already? */
 	if (CurrentLocaleConv)
 		return CurrentLocaleConv;
 
 	/* Save current locale setting to lc */
 	PGLC_current(&lc);
 
-	/* Set all locale category for current lang */
+	/* Set all locale categories based on postmaster's environment vars */
 	setlocale(LC_ALL, "");
 
-	/* Get numeric formatting information */
+	/* Get formatting information for the external environment */
 	CurrentLocaleConv = localeconv();
 
-	/* Set previous original locale */
+	/* Restore Postgres' internal locale settings */
 	PGLC_setlocale(&lc);
 
 	return CurrentLocaleConv;
 }
 
-
 #endif	 /* USE_LOCALE */

src/include/utils/pg_locale.h

@@ -1,13 +1,12 @@
-
 /* -----------------------------------------------------------------------
  * pg_locale.h
  *
- * $Header: /cvsroot/pgsql/src/include/utils/pg_locale.h,v 1.4 2000/04/12 17:16:55 momjian Exp $
+ *	 The PostgreSQL locale utils.
  *
  *
- *	 Portions Copyright (c) 1999-2000, PostgreSQL, Inc
+ * $Id: pg_locale.h,v 1.5 2000/11/25 22:43:07 tgl Exp $
  *
- *	 The PostgreSQL locale utils.
+ *	 Portions Copyright (c) 1999-2000, PostgreSQL, Inc
  *
  *	Karel Zak - Zakkr
  *
@@ -35,13 +34,13 @@ typedef struct PG_LocaleCategories
 }			PG_LocaleCategories;
 
 
-extern PG_LocaleCategories *PGLC_current(PG_LocaleCategories * lc);
-extern PG_LocaleCategories *PGLC_setlocale(PG_LocaleCategories * lc);
+extern void PGLC_current(PG_LocaleCategories * lc);
 
 /*------
  * Return the POSIX lconv struct (contains number/money formatting information)
- * with locale information for *all* categories. Returned lconv is *independent*
- * on current locale catogories setting - in contrast to standard localeconv().
+ * with locale information for all categories.  Note that returned lconv
+ * does not depend on currently active category settings, but on external
+ * environment variables for locale.
  *------
  */
 extern struct lconv *PGLC_localeconv(void);