On 13/08/2024 08:45, Thomas Munro wrote:
Hi,
Over on the discussion thread about remaining setlocale() work[1], I
wrote out some long boring theories about $SUBJECT. Here are some
draft patches to try those theories out, and make a commitfest entry.
nl_langinfo_l() is a trivial drop-in replacement, and
pg_localeconv_r() has 4 different implementation strategies:
1. Windows, with ugly _configthreadlocale() and thread-local result.
2. Glibc, with nice nl_langinfo_l() extensions.
3. macOS/*BSD, with nice localeconv_l().
4. Baseline POSIX: uselocale() + localeconv() + honking great lock.
In reality it'd just be Solaris running #4 (and AIX if it comes back).
Whether they truly implement it as pessimally as the standard allows,
who knows... you could drop the lock if you somehow knew that they
returned a pointer to thread-local storage or a member of the locale_t
object.
Patches 1 and 2 look good to me.
Patch 3 makes sense too, some comments on the details:
The #ifdefs and the LCONV_MEMBER stuff makes it a bit hard to follow
what happens in each implementation strategy. I wonder if it would be
more clear to duplicate more code.
There's a comment at the top of pg_locale.c ("!!! NOW HEAR THIS !!!")
that needs to be removed or adjusted now.
* The POSIX standard explicitly says that it is undefined what happens
if
* LC_MONETARY or LC_NUMERIC imply an encoding (codeset) different from
* that implied by LC_CTYPE. In practice, all Unix-ish platforms seem
to
* believe that localeconv() should return strings that are encoded in
the
* codeset implied by the LC_MONETARY or LC_NUMERIC locale name. Hence,
* once we have successfully collected the localeconv() results, we will
* convert them from that codeset to the desired server encoding.
The patch loses this comment, leaving just a much shorter comment in the
WIN32 implementation. But it still seems like a relevant comment for the
!WIN32 implementation too.
This gets rid of some setlocale() calls and makes the returned value
unclobberable with a defined lifetime. The remaining call to
setlocale() is only a query of the name of the current local (in a
typo: local -> locale
multi-threaded future this would have to be changed, perhaps to use a
per-database or per-backend locale_t instead of LC_GLOBAL_LOCALE).
All known non-Windows targets have nl_langinfo_l(), from POSIX 2018.
I think that's supposed to be POSIX 2008
--
Heikki Linnakangas
Neon (https://neon.tech)