From: Tristan Partin <tristan@neon.tech>
To: joe@cd
To: Heikki Linnakangas <hlinnaka@iki.fi>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: gdo@leader.it
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
Date: Mon, 03 Jul 2023 09:42:58 -0500
Message-ID: <CTSM7EZZMY12.1F7GV7Y2KPBBD@gonk> (raw)
In-Reply-To: <1645ebad-798f-87ad-43ac-72d2bf14d2d6@cd>
References: <17946-3e84cb577e9551c3@postgresql.org>
<1564676.1685043220@sss.pgh.pa.us>
<555bb26e-c27c-ab65-028f-3fd5b82df9de@iki.fi>
<CTPMCSM9UB07.3CUPU2JW4BBZ6@gonk>
<1645ebad-798f-87ad-43ac-72d2bf14d2d6@cd>
On Fri Jun 30, 2023 at 7:13 AM CDT, Joe Conway wrote:
> On 6/29/23 22:13, Tristan Partin wrote:
> > On Mon Jun 5, 2023 at 11:00 AM CDT, Heikki Linnakangas wrote:
> >> I think the uselocale() call renders ineffective the setlocale() calls
> >> that we make later. Maybe we should replace our setlocale() calls with
> >> uselocale(), too.
> >
> > For what it's worth to everyone else in the thread (especially Joe), I
> > have a patch locally that fixes the mentioned bug using uselocale(). I
> > am not sure that it is worth committing for v16 given how _large_ (the
> > patch is actually quite small, +216 -235) of a change it is. I am going
> > to spend tomorrow combing over it a bit more and evaluating other
> > setlocale uses in the codebase.
>
> (moving thread to hackers)
>
> I don't see a patch attached -- how is it different than what I posted a
> week ago and added to the commitfest here?
>
> https://commitfest.postgresql.org/43/4413/
>
> FWIW, if you are proposing replacing all uses of setlocale() with
> uselocale() as Heikki suggested:
>
> 1/ I don't think that is pg16 material, and almost certainly not
> back-patchable to earlier.
I am in agreement.
> 2/ It probably does not solve all of the identified issues caused by the
> newer perl libraries by itself, i.e. I believe the patch posted to the
> CF is still needed.
Perhaps. I do think your patch is still valuable regardless. Works for
backpatching and is just good defensive programming. I have added myself
as a reviewer.
> 3/ I believe it is probably the right way to go for pg17+, but I would
> love to hear opinions from Jeff Davis, Peter Eisentraut, and/or Thomas
> Munroe (the locale code "usual suspects" ;-)), and others, about that.
Thanks for your patience. Attached is a patch that should cover all the
problematic use cases of setlocale(). There are some setlocale() calls in
tests, initdb, and ecpg left. I plan to get to ecpglib before the final
version of this patch after I abstract over Windows not having
uselocale(). I think leaving initdb and tests as is would be fine, but I
am also happy to just permanently purge setlocale() from the codebase
if people see value in that. We could also poison[0] setlocale() at that
point.
[0]: https://gcc.gnu.org/onlinedocs/cpp/Pragmas.html
--
Tristan Partin
Neon (https://neon.tech)
Attachments:download all (3 files, .tar.gz)v1-0001-Skip-checking-for-uselocale-on-Windows.patch (text/x-patch, 677B) download
From 5688bc2b2c6331f437a72b6a429199c5416ccd76 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Fri, 30 Jun 2023 09:31:04 -0500
Subject: [PATCH v1 1/3] Skip checking for uselocale on Windows
Windows doesn't have uselocale, so skip it.
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.buildindex fbec997947..df76f10822 100644--- a/meson.build+++ b/meson.build@@ -2439,7 +2439,7 @@ func_checks = [
['strsignal'],
['sync_file_range'],
['syncfs'],
- ['uselocale'],+ ['uselocale', {'skip': host_system == 'windows'}],
['wcstombs_l'],
]
--
Tristan Partin
Neon (https://neon.tech)
v1-0002-Add-locale_is_c-function.patch (text/x-patch, 4.5K) download
From d52ab7851e9638e36cd99859014d7aa31154e600 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Fri, 30 Jun 2023 11:13:29 -0500
Subject: [PATCH v1 2/3] Add locale_is_c function
In some places throughout the codebase, there are string comparisons for
locales matching C or POSIX. Encapsulate this logic into a single
function and use it.
---
src/backend/utils/adt/pg_locale.c | 37 ++++++++++++++++++-------------
src/backend/utils/init/postinit.c | 4 +---
src/backend/utils/mb/mbutils.c | 5 ++---
src/include/utils/pg_locale.h | 1 +
4 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.cindex 514d4a9eeb..b455ef3aff 100644--- a/src/backend/utils/adt/pg_locale.c+++ b/src/backend/utils/adt/pg_locale.c@@ -154,6 +154,21 @@ static void icu_set_collation_attributes(UCollator *collator, const char *loc,
UErrorCode *status);
#endif
+/*+ * Check if a locale is the C locale+ *+ * POSIX is an alias for C. Passing ingore_case as true will use+ * pg_strcasecmp() instead of strcmp().+ */+bool+locale_is_c(const char *locale, bool ignore_case)+{+ if (!ignore_case)+ return strcmp(locale, "C") == 0 || strcmp(locale, "POSIX") == 0;++ return pg_strcasecmp(locale, "C") == 0 || pg_strcasecmp(locale, "POSIX") == 0;+}+
/*
* pg_perm_setlocale
*
@@ -1239,10 +1254,8 @@ lookup_collation_cache(Oid collation, bool set_flags)
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
collctype = TextDatumGetCString(datum);
- cache_entry->collate_is_c = ((strcmp(collcollate, "C") == 0) ||- (strcmp(collcollate, "POSIX") == 0));- cache_entry->ctype_is_c = ((strcmp(collctype, "C") == 0) ||- (strcmp(collctype, "POSIX") == 0));+ cache_entry->collate_is_c = locale_is_c(collcollate, false);+ cache_entry->ctype_is_c = locale_is_c(collctype, false);
}
else
{
@@ -1290,12 +1303,8 @@ lc_collate_is_c(Oid collation)
if (!localeptr)
elog(ERROR, "invalid LC_COLLATE setting");
- if (strcmp(localeptr, "C") == 0)- result = true;- else if (strcmp(localeptr, "POSIX") == 0)- result = true;- else- result = false;+ result = locale_is_c(localeptr, false);+
return (bool) result;
}
@@ -1343,12 +1352,8 @@ lc_ctype_is_c(Oid collation)
if (!localeptr)
elog(ERROR, "invalid LC_CTYPE setting");
- if (strcmp(localeptr, "C") == 0)- result = true;- else if (strcmp(localeptr, "POSIX") == 0)- result = true;- else- result = false;+ result = locale_is_c(localeptr, false);+
return (bool) result;
}
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.cindex 0f9b92b32e..a92a0c438f 100644--- a/src/backend/utils/init/postinit.c+++ b/src/backend/utils/init/postinit.c@@ -419,9 +419,7 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
" which is not recognized by setlocale().", ctype),
errhint("Recreate the database with another locale or install the missing locale.")));
- if (strcmp(ctype, "C") == 0 ||- strcmp(ctype, "POSIX") == 0)- database_ctype_is_c = true;+ database_ctype_is_c = locale_is_c(ctype, false);
if (dbform->datlocprovider == COLLPROVIDER_ICU)
{
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.cindex 67a1ab2ab2..997156515c 100644--- a/src/backend/utils/mb/mbutils.c+++ b/src/backend/utils/mb/mbutils.c@@ -39,6 +39,7 @@
#include "mb/pg_wchar.h"
#include "utils/builtins.h"
#include "utils/memutils.h"
+#include "utils/pg_locale.h"
#include "utils/syscache.h"
#include "varatt.h"
@@ -1237,9 +1238,7 @@ pg_bind_textdomain_codeset(const char *domainname)
int new_msgenc;
#ifndef WIN32
- const char *ctype = setlocale(LC_CTYPE, NULL);-- if (pg_strcasecmp(ctype, "C") == 0 || pg_strcasecmp(ctype, "POSIX") == 0)+ if (locale_is_c(locale_ctype, true))
#endif
if (encoding != PG_SQL_ASCII &&
raw_pg_bind_textdomain_codeset(domainname, encoding))
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.hindex e2a7243542..872bef798a 100644--- a/src/include/utils/pg_locale.h+++ b/src/include/utils/pg_locale.h@@ -54,6 +54,7 @@ extern PGDLLIMPORT bool database_ctype_is_c;
extern bool check_locale(int category, const char *locale, char **canonname);
extern char *pg_perm_setlocale(int category, const char *locale);
+extern bool locale_is_c(const char *locale, bool ignore_case);
extern bool lc_collate_is_c(Oid collation);
extern bool lc_ctype_is_c(Oid collation);
--
Tristan Partin
Neon (https://neon.tech)
v1-0003-Use-uselocale-instead-of-setlocale.patch (text/x-patch, 26.7K) download
From 75b5f7a4e4ecaf494302721b35c919d5e193b6d9 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tristan@neon.tech>
Date: Mon, 3 Jul 2023 08:51:59 -0500
Subject: [PATCH v1 3/3] Use uselocale() instead of setlocale()
setlocale() has been a thorn in Postgres's side for a while. Most
recently this has manifested in bug #17946 where loading libperl would
cause the localization to change out from under Postgres.
---
src/backend/main/main.c | 27 +-
src/backend/utils/adt/cash.c | 29 +++
src/backend/utils/adt/formatting.c | 11 +-
src/backend/utils/adt/pg_locale.c | 404 ++++++++++++++++-------------
src/backend/utils/init/postinit.c | 4 +-
src/backend/utils/mb/mbutils.c | 2 +-
src/common/exec.c | 16 --
src/include/utils/pg_locale.h | 2 +-
8 files changed, 275 insertions(+), 220 deletions(-)
diff --git a/src/backend/main/main.c b/src/backend/main/main.cindex ed11e8be7f..5b0e9fccda 100644--- a/src/backend/main/main.c+++ b/src/backend/main/main.c@@ -130,13 +130,6 @@ main(int argc, char *argv[])
init_locale("LC_NUMERIC", LC_NUMERIC, "C");
init_locale("LC_TIME", LC_TIME, "C");
- /*- * Now that we have absorbed as much as we wish to from the locale- * environment, remove any LC_ALL setting, so that the environment- * variables installed by pg_perm_setlocale have force.- */- unsetenv("LC_ALL");-
/*
* Catch standard options before doing much else, in particular before we
* insist on not being root.
@@ -307,8 +300,24 @@ startup_hacks(const char *progname)
static void
init_locale(const char *categoryname, int category, const char *locale)
{
- if (pg_perm_setlocale(category, locale) == NULL &&- pg_perm_setlocale(category, "C") == NULL)+ /* Since we are initializing global locales, NULL is an invalid argument. */+ Assert(locale);++ /*+ * uselocale() and friends don't have an equivalent to+ * setlocale(category, NULL), so we have to stash the actual locale string+ * in case it is needed. To do that, we canonicalize the empty string+ * argument here. pg_setlocale() asserts that it doesn't receive the empty+ * string.+ */+ if (locale[0] == '\0')+ locale = setlocale(category, locale);++ if (locale == NULL)+ elog(FATAL, "%s locale was not properly canonicalized", categoryname);++ if (pg_setlocale(category, locale) == NULL &&+ pg_setlocale(category, "C") == NULL)
elog(FATAL, "could not adopt \"%s\" locale nor C locale for %s",
locale, categoryname);
}
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.cindex 32fbad2f57..5c254d912f 100644--- a/src/backend/utils/adt/cash.c+++ b/src/backend/utils/adt/cash.c@@ -111,6 +111,11 @@ cash_in(PG_FUNCTION_ARGS)
*csymbol;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)+ ereturn(escontext, (Datum) 0,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("could not allocate memory for locale information")));+
/*
* frac_digits will be CHAR_MAX in some locales, notably C. However, just
* testing for == CHAR_MAX is risky, because of compilers like gcc that
@@ -325,6 +330,11 @@ cash_out(PG_FUNCTION_ARGS)
sep_by_space;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)+ ereturn(fcinfo->context, (Datum) 0,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("could not allocate memory for locale information")));+
/* see comments about frac_digits in cash_in() */
points = lconvert->frac_digits;
if (points < 0 || points > 10)
@@ -1036,6 +1046,11 @@ cash_numeric(PG_FUNCTION_ARGS)
int fpoint;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)+ ereturn(fcinfo->context, (Datum) 0,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("could not allocate memory for locale information")));+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1095,6 +1110,11 @@ numeric_cash(PG_FUNCTION_ARGS)
Datum numeric_scale;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)+ ereturn(fcinfo->context, (Datum) 0,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("could not allocate memory for locale information")));+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1128,6 +1148,11 @@ int4_cash(PG_FUNCTION_ARGS)
int i;
struct lconv *lconvert = PGLC_localeconv();
+ if (lconvert == NULL)+ ereturn(fcinfo->context, (Datum) 0,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("could not allocate memory for locale information")));+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
@@ -1158,6 +1183,10 @@ int8_cash(PG_FUNCTION_ARGS)
int i;
struct lconv *lconvert = PGLC_localeconv();
+ ereturn(fcinfo->context, (Datum) 0,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("could not allocate memory for locale information")));+
/* see comments about frac_digits in cash_in() */
fpoint = lconvert->frac_digits;
if (fpoint < 0 || fpoint > 10)
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.cindex e6246dc44b..b4229d075e 100644--- a/src/backend/utils/adt/formatting.c+++ b/src/backend/utils/adt/formatting.c@@ -5067,12 +5067,12 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Positive / Negative number sign
*/
- if (lconv->negative_sign && *lconv->negative_sign)+ if (lconv && lconv->negative_sign && *lconv->negative_sign)
Np->L_negative_sign = lconv->negative_sign;
else
Np->L_negative_sign = "-";
- if (lconv->positive_sign && *lconv->positive_sign)+ if (lconv && lconv->positive_sign && *lconv->positive_sign)
Np->L_positive_sign = lconv->positive_sign;
else
Np->L_positive_sign = "+";
@@ -5080,9 +5080,8 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Number decimal point
*/
- if (lconv->decimal_point && *lconv->decimal_point)+ if (lconv && lconv->decimal_point && *lconv->decimal_point)
Np->decimal = lconv->decimal_point;
-
else
Np->decimal = ".";
@@ -5096,7 +5095,7 @@ NUM_prepare_locale(NUMProc *Np)
* but "" for thousands_sep, so we set the thousands_sep too.
* http://archives.postgresql.org/pgsql-hackers/2007-11/msg00772.php
*/
- if (lconv->thousands_sep && *lconv->thousands_sep)+ if (lconv && lconv->thousands_sep && *lconv->thousands_sep)
Np->L_thousands_sep = lconv->thousands_sep;
/* Make sure thousands separator doesn't match decimal point symbol. */
else if (strcmp(Np->decimal, ",") != 0)
@@ -5107,7 +5106,7 @@ NUM_prepare_locale(NUMProc *Np)
/*
* Currency symbol
*/
- if (lconv->currency_symbol && *lconv->currency_symbol)+ if (lconv && lconv->currency_symbol && *lconv->currency_symbol)
Np->L_currency_symbol = lconv->currency_symbol;
else
Np->L_currency_symbol = " ";
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.cindex b455ef3aff..9b5d4384c6 100644--- a/src/backend/utils/adt/pg_locale.c+++ b/src/backend/utils/adt/pg_locale.c@@ -98,6 +98,21 @@ char *locale_time;
int icu_validation_level = WARNING;
+/*+ * Current locale settings+ *+ * uselocale() and friends don't have an equivalent to setlocale(cat, NULL), so+ * as we set locales for various categories, stash them for retrieval as needed.+ */+static struct {+ char *collate;+ char *ctype;+ char *messages;+ char *monetary;+ char *numeric;+ char *time;+} curr_locales = { 0 };+
/*
* lc_time localization cache.
*
@@ -169,50 +184,143 @@ locale_is_c(const char *locale, bool ignore_case)
return pg_strcasecmp(locale, "C") == 0 || pg_strcasecmp(locale, "POSIX") == 0;
}
+
/*
- * pg_perm_setlocale- *- * This wraps the libc function setlocale(), with two additions. First, when- * changing LC_CTYPE, update gettext's encoding for the current message- * domain. GNU gettext automatically tracks LC_CTYPE on most platforms, but- * not on Windows. Second, if the operation is successful, the corresponding- * LC_XXX environment variable is set to match. By setting the environment- * variable, we ensure that any subsequent use of setlocale(..., "") will- * preserve the settings made through this routine. Of course, LC_ALL must- * also be unset to fully ensure that, but that has to be done elsewhere after- * all the individual LC_XXX variables have been set correctly. (Thank you- * Perl for making this kluge necessary.)+ * Turns locale categories into their mask equivalents for use in newlocale(3)+ *+ * GCC has the masks defined as (1 << category), but this isn't guaranteed to be+ * the same in other libcs.+ */+static int+category_to_mask(int category)+{+ switch (category) {+ case LC_ALL:+ return LC_ALL_MASK;+ case LC_COLLATE:+ return LC_COLLATE_MASK;+ case LC_CTYPE:+ return LC_CTYPE_MASK;+#ifdef LC_MESSAGES+ case LC_MESSAGES:+ return LC_MESSAGES_MASK;+#endif+ case LC_MONETARY:+ return LC_MONETARY_MASK;+ case LC_NUMERIC:+ return LC_NUMERIC_MASK;+ case LC_TIME:+ return LC_TIME_MASK;+ }++ pg_unreachable();+}+++static char *+pg_getlocale(int category)+{+ switch (category) {+ case LC_COLLATE:+ return curr_locales.collate;+ case LC_CTYPE:+ return curr_locales.ctype;+ case LC_MESSAGES:+ return curr_locales.messages;+ case LC_MONETARY:+ return curr_locales.monetary;+ case LC_NUMERIC:+ return curr_locales.numeric;+ case LC_TIME:+ return curr_locales.time;+ default:+ pg_unreachable();+ }+}+++/*+ * pg_setlocale+ *+ * This wraps the libc function uselocale(), with a single addition. When+ * changing LC_CTYPE, update gettext's encoding for the current message domain.+ * GNU gettext automatically tracks LC_CTYPE on most platforms, but not on+ * Windows. This function does will abort if locale is the empty string. A+ * workaround is to pass the output of setlocale(category, "") to the locale+ * argument.
*/
char *
-pg_perm_setlocale(int category, const char *locale)+pg_setlocale(int category, const char *locale)
{
- char *result;- const char *envvar;+ char *dup;+ char **save;+ int category_mask;+ locale_t prev_locale;+ locale_t temp_locale;+ locale_t work_locale;-#ifndef WIN32- result = setlocale(category, locale);-#else+ if (locale == NULL)+ return pg_getlocale(category);++ Assert(locale[0] != '\0');++ category_mask = category_to_mask(category);++ prev_locale = uselocale((locale_t) 0);+ Assert(saved_locale != (locale_t) 0);++ dup = strdup(locale);+ work_locale = duplocale(prev_locale);+ if (!dup || work_locale == (locale_t) 0) {+ ereport(ERROR,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("out of memory")));++ return NULL;+ }++ temp_locale = newlocale(category_mask, locale, work_locale);+ if (temp_locale == (locale_t) 0) {+ freelocale(work_locale);++ return NULL; /* fall out immediately on failure */+ }+ work_locale = temp_locale;++ uselocale(work_locale);+ if (prev_locale != LC_GLOBAL_LOCALE)+ freelocale(prev_locale);
/*
- * On Windows, setlocale(LC_MESSAGES) does not work, so just assume that- * the given value is good and set it in the environment variables. We- * must ignore attempts to set to "", which means "keep using the old- * environment value".+ * Copying the locale must occur before we potentially bind the text domain.
*/
-#ifdef LC_MESSAGES- if (category == LC_MESSAGES)- {- result = (char *) locale;- if (locale == NULL || locale[0] == '\0')- return result;+ switch (category) {+ case LC_COLLATE:+ save = &curr_locales.collate;+ break;+ case LC_CTYPE:+ save = &curr_locales.ctype;+ break;+ case LC_MESSAGES:+ save = &curr_locales.messages;+ break;+ case LC_MONETARY:+ save = &curr_locales.monetary;+ break;+ case LC_NUMERIC:+ save = &curr_locales.numeric;+ break;+ case LC_TIME:+ save = &curr_locales.time;+ break;+ default:+ pg_unreachable();
}
- else-#endif- result = setlocale(category, locale);-#endif /* WIN32 */- if (result == NULL)- return result; /* fall out immediately on failure */+ /* Free old locale string, and stash the new one */+ free(*save);+ *save = dup;+ dup = NULL;
/*
* Use the right encoding in translated messages. Under ENABLE_NLS, let
@@ -223,12 +331,6 @@ pg_perm_setlocale(int category, const char *locale)
*/
if (category == LC_CTYPE)
{
- static char save_lc_ctype[LOCALE_NAME_BUFLEN];-- /* copy setlocale() return value before callee invokes it again */- strlcpy(save_lc_ctype, result, sizeof(save_lc_ctype));- result = save_lc_ctype;-
#ifdef ENABLE_NLS
SetMessageEncoding(pg_bind_textdomain_codeset(textdomain(NULL)));
#else
@@ -236,43 +338,7 @@ pg_perm_setlocale(int category, const char *locale)
#endif
}
- switch (category)- {- case LC_COLLATE:- envvar = "LC_COLLATE";- break;- case LC_CTYPE:- envvar = "LC_CTYPE";- break;-#ifdef LC_MESSAGES- case LC_MESSAGES:- envvar = "LC_MESSAGES";-#ifdef WIN32- result = IsoLocaleName(locale);- if (result == NULL)- result = (char *) locale;- elog(DEBUG3, "IsoLocaleName() executed; locale: \"%s\"", result);-#endif /* WIN32 */- break;-#endif /* LC_MESSAGES */- case LC_MONETARY:- envvar = "LC_MONETARY";- break;- case LC_NUMERIC:- envvar = "LC_NUMERIC";- break;- case LC_TIME:- envvar = "LC_TIME";- break;- default:- elog(FATAL, "unrecognized LC category: %d", category);- return NULL; /* keep compiler quiet */- }-- if (setenv(envvar, result, 1) != 0)- return NULL;-- return result;+ return *save;
}
@@ -289,32 +355,43 @@ pg_perm_setlocale(int category, const char *locale)
bool
check_locale(int category, const char *locale, char **canonname)
{
- char *save;- char *res;+ locale_t saved_locale;+ locale_t temp_locale;+ locale_t work_locale;+ int category_mask;++ category_mask = category_to_mask(category);
if (canonname)
*canonname = NULL; /* in case of failure */
- save = setlocale(category, NULL);- if (!save)- return false; /* won't happen, we hope */+ saved_locale = uselocale((locale_t) 0);+ work_locale = duplocale(saved_locale);+ if (work_locale == (locale_t) 0) {+ if (errno == ENOMEM)+ ereport(ERROR,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("out of memory")));++ return false;+ }- /* save may be pointing at a modifiable scratch variable, see above. */- save = pstrdup(save);+ temp_locale = newlocale(category_mask, locale, work_locale);+ if (temp_locale == (locale_t) 0) {+ freelocale(work_locale);- /* set the locale with setlocale, to see if it accepts it. */- res = setlocale(category, locale);+ return false;+ }+ work_locale = temp_locale;- /* save canonical name if requested. */- if (res && canonname)- *canonname = pstrdup(res);+ uselocale(work_locale);+ if (canonname)+ *canonname = pstrdup(locale); /* TODO: This does not handle "" as the locale */- /* restore old value. */- if (!setlocale(category, save))- elog(WARNING, "failed to restore old locale \"%s\"", save);- pfree(save);+ uselocale(saved_locale);+ freelocale(work_locale);- return (res != NULL);+ return true;
}
@@ -327,7 +404,7 @@ check_locale(int category, const char *locale, char **canonname)
*
* Note: we accept value = "" as selecting the postmaster's environment
* value, whatever it was (so long as the environment setting is legal).
- * This will have been locked down by an earlier call to pg_perm_setlocale.+ * This will have been locked down by an earlier call to pg_setlocale.
*/
bool
check_locale_monetary(char **newval, void **extra, GucSource source)
@@ -406,7 +483,7 @@ assign_locale_messages(const char *newval, void *extra)
* We ignore failure, as per comment above.
*/
#ifdef LC_MESSAGES
- (void) pg_perm_setlocale(LC_MESSAGES, newval);+ (void) pg_setlocale(LC_MESSAGES, newval);
#endif
}
@@ -502,11 +579,8 @@ PGLC_localeconv(void)
static bool CurrentLocaleConvAllocated = false;
struct lconv *extlconv;
struct lconv worklconv;
- char *save_lc_monetary;- char *save_lc_numeric;-#ifdef WIN32- char *save_lc_ctype;-#endif+ locale_t saved_locale;+ locale_t work_locale;
/* Did we do it already? */
if (CurrentLocaleConvValid)
@@ -533,16 +607,15 @@ PGLC_localeconv(void)
*/
memset(&worklconv, 0, sizeof(worklconv));
- /* Save prevailing values of monetary and numeric locales */- save_lc_monetary = setlocale(LC_MONETARY, NULL);- if (!save_lc_monetary)- elog(ERROR, "setlocale(NULL) failed");- save_lc_monetary = pstrdup(save_lc_monetary);+ /* Save prevailing locale */+ saved_locale = uselocale((locale_t) 0);+ Assert(saved_locale != (locale_t) 0);- save_lc_numeric = setlocale(LC_NUMERIC, NULL);- if (!save_lc_numeric)- elog(ERROR, "setlocale(NULL) failed");- save_lc_numeric = pstrdup(save_lc_numeric);+ work_locale = duplocale(saved_locale);+ if (work_locale == (locale_t) 0)+ ereport(ERROR,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("out of memory")));
#ifdef WIN32
@@ -560,37 +633,37 @@ PGLC_localeconv(void)
* results. Hence, we must temporarily set that category as well.
*/
- /* Save prevailing value of ctype locale */- save_lc_ctype = setlocale(LC_CTYPE, NULL);- if (!save_lc_ctype)- elog(ERROR, "setlocale(NULL) failed");- save_lc_ctype = pstrdup(save_lc_ctype);-
/* Here begins the critical section where we must not throw error */
/* use numeric to set the ctype */
- setlocale(LC_CTYPE, locale_numeric);+ work_locale = newlocale(LC_CTYPE_MASK, locale_numeric, work_locale);+ Assert(work_locale != (locale_t) 0);
#endif
/* Get formatting information for numeric */
- setlocale(LC_NUMERIC, locale_numeric);+ work_locale = newlocale(LC_NUMERIC_MASK, locale_numeric, work_locale);+ Assert(work_locale != (locale_t) 0);+ uselocale(work_locale);
extlconv = localeconv();
- /* Must copy data now in case setlocale() overwrites it */+ /* Must copy data now in case updating the locale overwrites it */
worklconv.decimal_point = strdup(extlconv->decimal_point);
worklconv.thousands_sep = strdup(extlconv->thousands_sep);
worklconv.grouping = strdup(extlconv->grouping);
#ifdef WIN32
/* use monetary to set the ctype */
- setlocale(LC_CTYPE, locale_monetary);+ work_locale = newlocale(LC_CTYPE_MASK, locale_monetary, work_locale);+ Assert(work_locale != (locale_t) 0);
#endif
/* Get formatting information for monetary */
- setlocale(LC_MONETARY, locale_monetary);+ work_locale = newlocale(LC_MONETARY_MASK, locale_monetary, work_locale);+ Assert(work_locale != (locale_t) 0);+ uselocale(work_locale);
extlconv = localeconv();
- /* Must copy data now in case setlocale() overwrites it */+ /* Must copy data now in case updating the locale overwrites it */
worklconv.int_curr_symbol = strdup(extlconv->int_curr_symbol);
worklconv.currency_symbol = strdup(extlconv->currency_symbol);
worklconv.mon_decimal_point = strdup(extlconv->mon_decimal_point);
@@ -608,22 +681,9 @@ PGLC_localeconv(void)
worklconv.p_sign_posn = extlconv->p_sign_posn;
worklconv.n_sign_posn = extlconv->n_sign_posn;
- /*- * Restore the prevailing locale settings; failure to do so is fatal.- * Possibly we could limp along with nondefault LC_MONETARY or LC_NUMERIC,- * but proceeding with the wrong value of LC_CTYPE would certainly be bad- * news; and considering that the prevailing LC_MONETARY and LC_NUMERIC- * are almost certainly "C", there's really no reason that restoring those- * should fail.- */-#ifdef WIN32- if (!setlocale(LC_CTYPE, save_lc_ctype))- elog(FATAL, "failed to restore LC_CTYPE to \"%s\"", save_lc_ctype);-#endif- if (!setlocale(LC_MONETARY, save_lc_monetary))- elog(FATAL, "failed to restore LC_MONETARY to \"%s\"", save_lc_monetary);- if (!setlocale(LC_NUMERIC, save_lc_numeric))- elog(FATAL, "failed to restore LC_NUMERIC to \"%s\"", save_lc_numeric);+ /* Restore the prevailing locale settings; failure to do so is fatal. */+ uselocale(saved_locale);+ freelocale(work_locale);
/*
* At this point we've done our best to clean up, and can call functions
@@ -634,13 +694,6 @@ PGLC_localeconv(void)
{
int encoding;
- /* Release the pstrdup'd locale names */- pfree(save_lc_monetary);- pfree(save_lc_numeric);-#ifdef WIN32- pfree(save_lc_ctype);-#endif-
/* If any of the preceding strdup calls failed, complain now. */
if (!struct_lconv_is_valid(&worklconv))
ereport(ERROR,
@@ -787,10 +840,8 @@ cache_locale_time(void)
bool strftimefail = false;
int encoding;
int i;
- char *save_lc_time;-#ifdef WIN32- char *save_lc_ctype;-#endif+ locale_t saved_locale;+ locale_t work_locale;
/* did we do this already? */
if (CurrentLCTimeValid)
@@ -805,11 +856,15 @@ cache_locale_time(void)
* results afterwards.
*/
- /* Save prevailing value of time locale */- save_lc_time = setlocale(LC_TIME, NULL);- if (!save_lc_time)- elog(ERROR, "setlocale(NULL) failed");- save_lc_time = pstrdup(save_lc_time);+ /* Save prevailing locale */+ saved_locale = uselocale((locale_t) 0);+ Assert(saved_locale != (locale_t) 0);++ work_locale = duplocale(saved_locale);+ if (work_locale == (locale_t) 0)+ ereport(ERROR,+ (errcode(ERRCODE_OUT_OF_MEMORY),+ errmsg("out of memory")));
#ifdef WIN32
@@ -820,17 +875,14 @@ cache_locale_time(void)
* the encoding we'll get back from strftime_win32().
*/
- /* Save prevailing value of ctype locale */- save_lc_ctype = setlocale(LC_CTYPE, NULL);- if (!save_lc_ctype)- elog(ERROR, "setlocale(NULL) failed");- save_lc_ctype = pstrdup(save_lc_ctype);-
/* use lc_time to set the ctype */
- setlocale(LC_CTYPE, locale_time);+ work_locale = newlocale(LC_CTYPE_MASK, locale_time, work_locale);+ Assert(work_locale != (locale_t) 0);
#endif
- setlocale(LC_TIME, locale_time);+ work_locale = newlocale(LC_TIME_MASK, locale_time, work_locale);+ Assert(work_locale != (locale_t) 0);+ uselocale(work_locale);
/* We use times close to current time as data for strftime(). */
timenow = time(NULL);
@@ -878,12 +930,8 @@ cache_locale_time(void)
* Restore the prevailing locale settings; as in PGLC_localeconv(),
* failure to do so is fatal.
*/
-#ifdef WIN32- if (!setlocale(LC_CTYPE, save_lc_ctype))- elog(FATAL, "failed to restore LC_CTYPE to \"%s\"", save_lc_ctype);-#endif- if (!setlocale(LC_TIME, save_lc_time))- elog(FATAL, "failed to restore LC_TIME to \"%s\"", save_lc_time);+ uselocale(saved_locale);+ freelocale(work_locale);
/*
* At this point we've done our best to clean up, and can throw errors, or
@@ -892,12 +940,6 @@ cache_locale_time(void)
if (strftimefail)
elog(ERROR, "strftime() failed: %m");
- /* Release the pstrdup'd locale names */- pfree(save_lc_time);-#ifdef WIN32- pfree(save_lc_ctype);-#endif-
#ifndef WIN32
/*
@@ -1292,18 +1334,14 @@ lc_collate_is_c(Oid collation)
if (collation == DEFAULT_COLLATION_OID)
{
static int result = -1;
- char *localeptr;
if (default_locale.provider == COLLPROVIDER_ICU)
return false;
if (result >= 0)
return (bool) result;
- localeptr = setlocale(LC_COLLATE, NULL);- if (!localeptr)- elog(ERROR, "invalid LC_COLLATE setting");- result = locale_is_c(localeptr, false);+ result = locale_is_c(curr_locales.collate, false);
return (bool) result;
}
@@ -1336,23 +1374,19 @@ lc_ctype_is_c(Oid collation)
/*
* If we're asked about the default collation, we have to inquire of the C
- * library. Cache the result so we only have to compute it once.+ * library.
*/
if (collation == DEFAULT_COLLATION_OID)
{
static int result = -1;
- char *localeptr;
if (default_locale.provider == COLLPROVIDER_ICU)
return false;
if (result >= 0)
return (bool) result;
- localeptr = setlocale(LC_CTYPE, NULL);- if (!localeptr)- elog(ERROR, "invalid LC_CTYPE setting");- result = locale_is_c(localeptr, false);+ result = locale_is_c(curr_locales.ctype, false);
return (bool) result;
}
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.cindex a92a0c438f..0741f5666b 100644--- a/src/backend/utils/init/postinit.c+++ b/src/backend/utils/init/postinit.c@@ -405,14 +405,14 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect
datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datctype);
ctype = TextDatumGetCString(datum);
- if (pg_perm_setlocale(LC_COLLATE, collate) == NULL)+ if (pg_setlocale(LC_COLLATE, collate) == NULL)
ereport(FATAL,
(errmsg("database locale is incompatible with operating system"),
errdetail("The database was initialized with LC_COLLATE \"%s\", "
" which is not recognized by setlocale().", collate),
errhint("Recreate the database with another locale or install the missing locale.")));
- if (pg_perm_setlocale(LC_CTYPE, ctype) == NULL)+ if (pg_setlocale(LC_CTYPE, ctype) == NULL)
ereport(FATAL,
(errmsg("database locale is incompatible with operating system"),
errdetail("The database was initialized with LC_CTYPE \"%s\", "
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.cindex 997156515c..8dc7da47bd 100644--- a/src/backend/utils/mb/mbutils.c+++ b/src/backend/utils/mb/mbutils.c@@ -1238,7 +1238,7 @@ pg_bind_textdomain_codeset(const char *domainname)
int new_msgenc;
#ifndef WIN32
- if (locale_is_c(locale_ctype, true))+ if (locale_is_c(pg_setlocale(LC_CTYPE, NULL), true))
#endif
if (encoding != PG_SQL_ASCII &&
raw_pg_bind_textdomain_codeset(domainname, encoding))
diff --git a/src/common/exec.c b/src/common/exec.cindex f209b934df..f7a9643be4 100644--- a/src/common/exec.c+++ b/src/common/exec.c@@ -438,22 +438,6 @@ set_pglocale_pgservice(const char *argv0, const char *app)
char path[MAXPGPATH];
char my_exec_path[MAXPGPATH];
- /* don't set LC_ALL in the backend */- if (strcmp(app, PG_TEXTDOMAIN("postgres")) != 0)- {- setlocale(LC_ALL, "");-- /*- * One could make a case for reproducing here PostmasterMain()'s test- * for whether the process is multithreaded. Unlike the postmaster,- * no frontend program calls sigprocmask() or otherwise provides for- * mutual exclusion between signal handlers. While frontends using- * fork(), if multithreaded, are formally exposed to undefined- * behavior, we have not witnessed a concrete bug. Therefore,- * complaining about multithreading here may be mere pedantry.- */- }-
if (find_my_exec(argv0, my_exec_path) < 0)
return;
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.hindex 872bef798a..60682051cb 100644--- a/src/include/utils/pg_locale.h+++ b/src/include/utils/pg_locale.h@@ -52,7 +52,7 @@ extern PGDLLIMPORT char *localized_full_months[];
extern PGDLLIMPORT bool database_ctype_is_c;
extern bool check_locale(int category, const char *locale, char **canonname);
-extern char *pg_perm_setlocale(int category, const char *locale);+extern char *pg_setlocale(int category, const char *locale);
extern bool locale_is_c(const char *locale, bool ignore_case);
extern bool lc_collate_is_c(Oid collation);
--
Tristan Partin
Neon (https://neon.tech)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: tristan@neon.tech, joe@cd, hlinnaka@iki.fi, tgl@sss.pgh.pa.us, gdo@leader.it, pgsql-hackers@postgresql.org
Subject: Re: BUG #17946: LC_MONETARY & DO LANGUAGE plperl - BUG
In-Reply-To: <CTSM7EZZMY12.1F7GV7Y2KPBBD@gonk>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox