public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Eisentraut <[email protected]>
To: Thomas Munro <[email protected]>
Cc: Tristan Partin <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: On non-Windows, hard depend on uselocale(3)
Date: Wed, 28 Aug 2024 20:50:31 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+hUKG+ue0KC5KSqMuL5zi7uCSuXCLP_B+sZSq5=yS84606ZsA@mail.gmail.com>
References: <[email protected]>
<CA+hUKGKo7Q4zayx37JxMaD-BzmkaW9j+k70bhA9auN9jztFrVQ@mail.gmail.com>
<[email protected]>
<CA+hUKGKu7H7iPNQsZ5FcPkZVbgXc5iTBgUjdpM2EHP4q=Tz42Q@mail.gmail.com>
<[email protected]>
<CA+hUKGJ4s6uDK_KYK-2q=2aWztgvTHYqGeSzugaTCEs6_3XqAg@mail.gmail.com>
<[email protected]>
<CA+hUKGLLb1MiqAUwtSb6gQsqm8wgAh+7UEE-6P1tQxjhHCFmrw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CA+hUKGLy2AeE0a6MHq_KQhJXbOQJ6UHsrUoCwb5OGj+WijAWqw@mail.gmail.com>
<[email protected]>
<CA+hUKG+YxfUyUOY7OHUFCgenAgq9znksd=Z+vyb20KHG=2iAnw@mail.gmail.com>
<[email protected]>
<CA+hUKG+Yv+ps=nS2T8SS1UDU=iySHSr4sGHYiYGkPTpZx6Ooww@mail.gmail.com>
<CA+hUKGLs9ccA7pUfphZOuf_fcVq2FAai7Cm0pmbQk0Qcs4VoVQ@mail.gmail.com>
<CA+hUKGKOuA4vTkFk01RSLqyM0pzNjYm-3h2zAtW48Ffc7FjjNg@mail.gmail.com>
<CA+hUKG+ue0KC5KSqMuL5zi7uCSuXCLP_B+sZSq5=yS84606ZsA@mail.gmail.com>
On 11.08.24 00:11, Thomas Munro wrote:
> v4 adds error handling, in case newlocale("C") fails. I created CF
> entry #5166 for this.
I took a look at this. It was quite a complicated discussion that led
to this, but I agree with the solution that was arrived at.
I suggest that the simplification of the xlocale.h configure tests could
be committed separately. This would also be useful independent of this,
and it's a sizeable chunk of this patch.
Also, you're removing the configure test for _configthreadlocale().
Presumably because you're removing all the uses. But wouldn't we need
that back later in the backend maybe? Or is that test even relevant
anymore, that is, are there Windows versions that don't have it?
Adding global includes to port.h doesn't seem great. That's not a place
one would normally look. We already include <locale.h> in c.h anyway,
so it would probably be even better overall if you just added a
conditional #include <xlocale.h> to c.h as well.
For Windows, we already have things like
#define strcoll_l _strcoll_l
in src/include/port/win32_port.h, so it would seem more sensible to add
strtod_l to that list, instead of in port.h.
The error handling with pg_ensure_c_locale(), that's the sort of thing
I'm afraid will be hard to test or even know how it will behave. And it
creates this weird coupling between pgtypeslib and ecpglib that you
mentioned earlier. And if there are other users of PG_C_LOCALE in the
future, there will be similar questions about the proper initialization
and error handling sequence.
I would consider instead making a local static variable in each function
that needs this. For example, numericvar_to_double() might do
{
static locale_t c_locale;
if (!c_locale)
{
c_locale = pg_get_c_locale();
if (!c_locale)
return -1; /* local error reporting convention */
}
...
}
This is a bit more code in total, but then you only initialize what you
need and you can handle errors locally.
view thread (32+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: On non-Windows, hard depend on uselocale(3)
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox