Re: check_strxfrm_bug()
Tristan Partin <tristan@neon.tech>
From: "Tristan Partin" <tristan@neon.tech>
To: "Thomas Munro" <thomas.munro@gmail.com>, "Peter Eisentraut"
<peter@eisentraut.org>
Cc: "Noah Misch" <noah@leadboat.com>, "Heikki Linnakangas"
<hlinnaka@iki.fi>, "pgsql-hackers" <pgsql-hackers@postgresql.org>
Date: 2023-07-07T20:52:36Z
Lists: pgsql-hackers
On Fri Jul 7, 2023 at 3:30 PM CDT, Thomas Munro wrote:
> On Fri, Jul 7, 2023 at 4:20 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> > I think the correct solution is to set HAVE_MBSTOWCS_L in Solution.pm.
> > Compare HAVE_FSEEKO, which is set in Solution.pm with fseeko being
> > defined in win32_port.h.
>
> In this version I have it in there, but set it to undef. This way
> configure (MinGW), meson (MinGW or MSVC), and Solution.pm all agree.
> This version passes on CI (not quite sure what I screwed up before).
>
> To restate the problem I am solving with this Solution.pm change +
> associated changes in the C: configure+MinGW disabled the libc
> provider completely before. With this patch that is fixed, and that
> forced me to address the inconsistency (because if you have the libc
> provider but no _l functions, you hit uselocale() code paths that
> Windows can't do). It was a bug, really, but I don't plan to
> back-patch anything (nobody really uses configure+MinGW builds AFAIK,
> they exist purely as a developer [in]convenience). But that explains
> why I needed to make a change.
>
> Thinking about how to bring this all into "normal" form -- where
> HAVE_XXX means "system defines XXX", not "system defines XXX || we
> define a replacement" -- leads to the attached. Do you like this one?
Should you wrap the two _l function replacements in HAVE_USELOCALE
instead of WIN32?
> +if not cc.has_type('locale_t', prefix: '#include <locale.h>') and cc.has_type('locale_t', prefix: '#include <xlocale.h>')
I wouldn't mind a line break after the 'and'.
Other than these comments, the patch looks fine to me.
--
Tristan Partin
Neon (https://neon.tech)
Commits
-
Don't expose Windows' mbstowcs_l() and wcstombs_l().
- 4e9fa6d56b3e 17.0 landed
-
All supported systems have locale_t.
- 8d9a9f034e92 17.0 landed
-
Remove obsolete defense against strxfrm() bugs.
- 7d3d72b55edd 16.0 landed