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

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

  1. Fix "‘static’ is not at beginning of declaration" warning

  2. Use thread-safe strftime_l() instead of strftime().

  3. Provide thread-safe pg_localeconv_r().

  4. Harmonize MinGW CODESET lookup with MSVC.

  5. Include <xlocale.h> for macOS, take II.

  6. Use thread-safe nl_langinfo_l(), not nl_langinfo().

  7. All POSIX systems have langinfo.h and CODESET.