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)
