Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Mon, 03 Jul 2023 09:49:21 -0500
Message-Id: <CTSMCH8UIGRM.Y7K2SSPBYWC4@gonk>
Cc: <pgsql-hackers@postgresql.org>
Subject: Re: Make uselocale protection more consistent
To: "Peter Eisentraut" <peter@eisentraut.org>, "Thomas Munro"
 <thomas.munro@gmail.com>
From: "Tristan Partin" <tristan@neon.tech>
References: <CTNITWIN1G6L.CFIPH8SYNQQW@gonk>
 <fe6fafd1-8bf3-9307-b1d0-e5074722452a@eisentraut.org>
 <CA+hUKGKkfh3wSC-J2eXd4kb499MCJZCCyiL61-mk641uzTBBAA@mail.gmail.com>
 <CTSKHGN8BPUK.14RF8P15GN4ZM@gonk>
 <14f5200e-388d-66ff-9deb-695d1fd40d68@eisentraut.org>
In-Reply-To: <14f5200e-388d-66ff-9deb-695d1fd40d68@eisentraut.org>
Archived-At: <https://www.postgresql.org/message-id/CTSMCH8UIGRM.Y7K2SSPBYWC4%40gonk>
Precedence: bulk

On Mon Jul 3, 2023 at 9:21 AM CDT, Peter Eisentraut wrote:
> On 03.07.23 15:21, Tristan Partin wrote:
> >>> I think it would be better to keep HAVE_LOCALE_T as encompassing any =
of
> >>> the various locale_t-using functions, rather than using HAVE_USELOCAL=
E
> >>> as a proxy for them.  Otherwise you create weird situations like havi=
ng
> >>> #ifdef HAVE_WCSTOMBS_L inside #ifdef HAVE_USELOCALE, which doesn't ma=
ke
> >>> sense, I think.
> >> I propose[1] that we get rid of HAVE_LOCALE_T completely and make
> >> "libc" provider support unconditional.  It's standardised, and every
> >> target system has it, even Windows.  But Windows doesn't have
> >> uselocale().
> >>
> >> [1]https://www.postgresql.org/message-id/flat/CA%2BhUKGL7CmmzeRhoirzjE=
CmOdABVFTn8fo6gEOaFRF1Oxey6Hw%40mail.gmail.com#aef2f2274b28ff8a36f9b8a598e3=
cec0
> > I think keeping HAVE_USELOCALE is important for the Windows case as
> > mentioned. I need it for my localization work where I am ripping out
> > setlocale() on non-Windows.
>
> The current code is structured
>
> #ifdef HAVE_LOCALE_T
> #ifdef HAVE_WCSTOMBS_L
>      wcstombs_l(...);
> #else
>      uselocale(...);
> #endif
> #else
>      elog(ERROR);
> #endif
>
> If you just replace HAVE_LOCALE_T with HAVE_USELOCALE, then this would=20
> penalize a platform that has wcstombs_l(), but not uselocale().  I think=
=20
> the correct structure would be
>
> #if defined(HAVE_WCSTOMBS_L)
>      wcstombs_l(...);
> #elif defined(HAVE_USELOCALE)
>      uselocale(...);
> #else
>      elog(ERROR);
> #endif

That makes sense to me. I gave it some more thought. Maybe it makes more
sense to just completely drop HAVE_USELOCALE as mentioned, and protect
calls to it with #ifdef WIN32 or whatever the macro is. HAVE_USELOCALE
might be more descriptive, but I don't really care that much either way.

--=20
Tristan Partin
Neon (https://neon.tech)