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)
From 5688bc2b2c6331f437a72b6a429199c5416ccd76 Mon Sep 17 00:00:00 2001 From: Tristan Partin <tris...@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.build index 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)
From d52ab7851e9638e36cd99859014d7aa31154e600 Mon Sep 17 00:00:00 2001 From: Tristan Partin <tris...@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.c index 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.c index 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.c index 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.h index 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)
From 75b5f7a4e4ecaf494302721b35c919d5e193b6d9 Mon Sep 17 00:00:00 2001 From: Tristan Partin <tris...@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.c index 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.c index 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.c index 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.c index 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.c index 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.c index 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.c index 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.h index 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)