Subject: Make uselocale protection more consistent

From: "Tristan Partin" <[email-masked]>
Date: Tue, 27 Jun 2023 10:02:05 -0500

This is a patch which implements an issue discussed in bug #17946[0]. It
doesn't fix the overarching issue of the bug, but merely a consistency
issue which was found while analyzing code by Heikki. I had originally
submitted the patch within that thread, but for visibility and the
purposes of the commitfest, I have re-sent it in its own thread.

[0]: https://www.postgresql.org/message-id/[email-masked]

-- 
Tristan Partin
Neon (https://neon.tech)
From 02a2cdb83405b5aecaec2af02e379a81161f8372 Mon Sep 17 00:00:00 2001
From: Tristan Partin <[email-masked]>
Date: Wed, 14 Jun 2023 11:36:00 -0500
Subject: [PATCH v1] Make uselocale protection more consistent

In ecpg, uselocale uses are protected by checking if HAVE_USELOCALE is
defined. Use the same check in pg_locale.c. Since HAVE_USELOCALE implies
HAVE_LOCALE_T, the code should be the same on _all_ platforms that
Postgres supports. Otherwise, I am sure there would have been a bug
report with pg_locale.c failing to build due to the system having
locale_t, but not uselocale.
---
 src/backend/utils/adt/pg_locale.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 31e3b16ae0..3585afb298 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -2972,7 +2972,7 @@ wchar2char(char *to, const wchar_t *from, size_t tolen, pg_locale_t locale)
 	}
 	else
 	{
-#ifdef HAVE_LOCALE_T
+#ifdef HAVE_USELOCALE
 #ifdef HAVE_WCSTOMBS_L
 		/* Use wcstombs_l for nondefault locales */
 		result = wcstombs_l(to, from, tolen, locale->info.lt);
@@ -2984,11 +2984,11 @@ wchar2char(char *to, const wchar_t *from, size_t tolen, pg_locale_t locale)
 
 		uselocale(save_locale);
 #endif							/* HAVE_WCSTOMBS_L */
-#else							/* !HAVE_LOCALE_T */
-		/* Can't have locale != 0 without HAVE_LOCALE_T */
+#else							/* !HAVE_USELOCALE */
+		/* Can't have locale != 0 without HAVE_LOCALE_T, which HAVE_USELOCALE implies */
 		elog(ERROR, "wcstombs_l is not available");
 		result = 0;				/* keep compiler quiet */
-#endif							/* HAVE_LOCALE_T */
+#endif							/* HAVE_USELOCALE */
 	}
 
 	return result;
@@ -3049,7 +3049,7 @@ char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen,
 		}
 		else
 		{
-#ifdef HAVE_LOCALE_T
+#ifdef HAVE_USELOCALE
 #ifdef HAVE_MBSTOWCS_L
 			/* Use mbstowcs_l for nondefault locales */
 			result = mbstowcs_l(to, str, tolen, locale->info.lt);
@@ -3061,11 +3061,11 @@ char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen,
 
 			uselocale(save_locale);
 #endif							/* HAVE_MBSTOWCS_L */
-#else							/* !HAVE_LOCALE_T */
-			/* Can't have locale != 0 without HAVE_LOCALE_T */
+#else							/* !HAVE_USELOCALE */
+			/* Can't have locale != 0 without HAVE_LOCALE_T, which HAVE_USELOCALE implies */
 			elog(ERROR, "mbstowcs_l is not available");
 			result = 0;			/* keep compiler quiet */
-#endif							/* HAVE_LOCALE_T */
+#endif							/* HAVE_USELOCALE */
 		}
 
 		pfree(str);
-- 
Tristan Partin
Neon (https://neon.tech)

---

From: Peter Eisentraut <[email-masked]>
Date: Mon, 3 Jul 2023 08:13:40 +0200

> [0]: https://www.postgresql.org/message-id/[email-masked]

I notice that HAVE_USELOCALE was introduced much later than 
HAVE_LOCALE_T, and at the time the code was already using uselocale(), 
so perhaps the introduction of HAVE_USELOCALE was unnecessary and should 
be reverted.

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_USELOCALE 
as a proxy for them.  Otherwise you create weird situations like having 
#ifdef HAVE_WCSTOMBS_L inside #ifdef HAVE_USELOCALE, which doesn't make 
sense, I think.

---

From: Thomas Munro <[email-masked]>
Date: Mon, 3 Jul 2023 18:24:02 +1200

> #ifdef HAVE_WCSTOMBS_L inside #ifdef HAVE_USELOCALE, which doesn't make

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%2BhUKGL7CmmzeRhoirzjECmOdABVFTn8fo6gEOaFRF1Oxey6Hw%40mail.gmail.com#aef2f2274b28ff8a36f9b8a598e3cec0

---

From: "Tristan Partin" <[email-masked]>
Date: Mon, 03 Jul 2023 08:21:50 -0500

> [1] https://www.postgresql.org/message-id/flat/CA%2BhUKGL7CmmzeRhoirzjECmOdABVFTn8fo6gEOaFRF1Oxey6Hw%40mail.gmail.com#aef2f2274b28ff8a36f9b8a598e3cec0

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.

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