Re: Thread-safe nl_langinfo() and localeconv()
Peter Eisentraut <peter@eisentraut.org>
From: Peter Eisentraut <peter@eisentraut.org>
To: Thomas Munro <thomas.munro@gmail.com>,
Heikki Linnakangas <hlinnaka@iki.fi>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-02-14T14:13:39Z
Lists: pgsql-hackers
Attachments
- v6.1-0001-Provide-thread-safe-pg_localeconv_r.patch (text/plain) patch v6-0001
- v6.1-0002-Use-thread-safe-strftime_l-instead-of-strftime.patch (text/plain) patch v6-0002
- v6.1-0003-Remove-setlocale-from-check_locale.patch (text/plain) patch v6-0003
- v6.1-0004-fixup-Provide-thread-safe-pg_localeconv_r.patch (text/plain) patch v6-0004
- v6.1-0005-fixup-Use-thread-safe-strftime_l-instead-of-str.patch (text/plain) patch v6-0005
On 09.02.25 15:52, Peter Eisentraut wrote: > This patch set is still desirable. Here is a rebased version of the v5 > patches. I haven't applied any corrections or review comments. Here is the same patch set with some review comments. Patch 0001 looks okay to me. I'm just offering some cosmetic improvements in patch 0004. Patch 0002 also looks okay, except that the error handling could be unified with existing code, as I had previously pointed out. Patch 0005 fixes that. About patch 0003: I had previously pointed out that the canonicalization might have been intentional, and that we have canonicalization of ICU locale names. But we don't have to keep the setlocale()-based locale checking implementation just for that, I think. (If this was meant to be a real feature offered by libc, there should be a get_locale_name(locale_t) function.) I'm unsure about the correct error handling of _create_locale() on Windows. Does _dosmaperr(GetLastError()) do anything useful in this context, or is this just copied from elsewhere? If it's useful, maybe it should be added to report_newlocale_failure(). Also, maybe we don't need per-category locale checking? Would it not be enough to check the locale using LC_ALL_MASK? Is there a scenario where a locale name would work for one category but not another? I think the old code was just conveniently coded that way so that you only have to save and restore one locale category. But we wouldn't have to do it that way anymore if we use newlocale().
Commits
-
Fix "‘static’ is not at beginning of declaration" warning
- 116e851db5b3 18.0 landed
-
Use thread-safe strftime_l() instead of strftime().
- 890fc826c91f 18.0 landed
-
Provide thread-safe pg_localeconv_r().
- b98be8a2a2a6 18.0 landed
-
Harmonize MinGW CODESET lookup with MSVC.
- bf3401fe813a 18.0 landed
-
Include <xlocale.h> for macOS, take II.
- 2488058dc356 18.0 landed
-
Use thread-safe nl_langinfo_l(), not nl_langinfo().
- 35eeea623022 18.0 landed
-
All POSIX systems have langinfo.h and CODESET.
- 14c648ff0094 18.0 landed