Skip to content

Commit 8e993bf

Browse files
peterepetere
committed
Tidy up locale thread safety in ECPG library.
Remove setlocale() and _configthreadlocal() as fallback strategy on systems that don't have uselocale(), where ECPG tries to control LC_NUMERIC formatting on input and output of floating point numbers. It was probably broken on some systems (NetBSD), and the code was also quite messy and complicated, with obsolete configure tests (Windows). It was also arguably broken, or at least had unstated environmental requirements, if pgtypeslib code was called directly. Instead, introduce PG_C_LOCALE to refer to the "C" locale as a locale_t value. It maps to the special constant LC_C_LOCALE when defined by libc (macOS, NetBSD), or otherwise uses a process-lifetime locale_t that is allocated on first use, just as ECPG previously did itself. The new replacement might be more widely useful. Then change the float parsing and printing code to pass that to _l() functions where appropriate. Unfortunately the portability of those functions is a bit complicated. First, many obvious and useful _l() functions are missing from POSIX, though most standard libraries define some of them anyway. Second, although the thread-safe save/restore technique can be used to replace the missing ones, Windows and NetBSD refused to implement standard uselocale(). They might have a point: "wide scope" uselocale() is hard to combine with other code and error-prone, especially in library code. Luckily they have the _l() functions we want so far anyway. So we have to be prepared for both ways of doing things: 1. In ECPG, use strtod_l() for parsing, and supply a port.h replacement using uselocale() over a limited scope if missing. 2. Inside our own snprintf.c, use three different approaches to format floats. For frontend code, call libc's snprintf_l(), or wrap libc's snprintf() in uselocale() if it's missing. For backend code, snprintf.c can keep assuming that the global locale's LC_NUMERIC is "C" and call libc's snprintf() without change, for now. (It might eventually be possible to call our in-tree Ryū routines to display floats in snprintf.c, given the C-locale-always remit of our in-tree snprintf(), but this patch doesn't risk changing anything that complicated.) Author: Thomas Munro <thomas.munro@gmail.com> Reviewed-by: Peter Eisentraut <peter@eisentraut.org> Reviewed-by: Tristan Partin <tristan@partin.io> Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
1 parent 2247281 commit 8e993bf

18 files changed

+192
-148
lines changed

‎configure

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15401,7 +15401,7 @@ fi
1540115401
LIBS_including_readline="$LIBS"
1540215402
LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
1540315403

15404-
for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l
15404+
for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast snprintf_l strchrnul strsignal strtod_l syncfs sync_file_range uselocale wcstombs_l
1540515405
do :
1540615406
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
1540715407
ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"

‎configure.ac

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1772,8 +1772,10 @@ AC_CHECK_FUNCS(m4_normalize([
17721772
pthread_is_threaded_np
17731773
setproctitle
17741774
setproctitle_fast
1775+
snprintf_l
17751776
strchrnul
17761777
strsignal
1778+
strtod_l
17771779
syncfs
17781780
sync_file_range
17791781
uselocale

‎meson.build

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2753,6 +2753,7 @@ func_checks = [
27532753
['shm_open', {'dependencies': [rt_dep], 'define': false}],
27542754
['shm_unlink', {'dependencies': [rt_dep], 'define': false}],
27552755
['shmget', {'dependencies': [cygipc_dep], 'define': false}],
2756+
['snprintf_l'],
27562757
['socket', {'dependencies': [socket_dep], 'define': false}],
27572758
['strchrnul'],
27582759
['strerror_r', {'dependencies': [thread_dep]}],
@@ -2761,6 +2762,7 @@ func_checks = [
27612762
['strnlen'],
27622763
['strsep'],
27632764
['strsignal'],
2765+
['strtod_l'],
27642766
['sync_file_range'],
27652767
['syncfs'],
27662768
['uselocale'],

‎src/include/pg_config.h.in

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,9 @@
355355
/* Define to 1 if you have the `setproctitle_fast' function. */
356356
#undef HAVE_SETPROCTITLE_FAST
357357

358+
/* Define to 1 if you have the `snprintf_l' function. */
359+
#undef HAVE_SNPRINTF_L
360+
358361
/* Define to 1 if the system has the type `socklen_t'. */
359362
#undef HAVE_SOCKLEN_T
360363

@@ -400,6 +403,9 @@
400403
/* Define to 1 if you have the `strsignal' function. */
401404
#undef HAVE_STRSIGNAL
402405

406+
/* Define to 1 if you have the `strtod_l' function. */
407+
#undef HAVE_STRTOD_L
408+
403409
/* Define to 1 if the system has the type `struct option'. */
404410
#undef HAVE_STRUCT_OPTION
405411

‎src/include/port.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,37 @@ extern int pg_fprintf(FILE *stream, const char *fmt,...) pg_attribute_printf(2,
218218
extern int pg_vprintf(const char *fmt, va_list args) pg_attribute_printf(1, 0);
219219
extern int pg_printf(const char *fmt,...) pg_attribute_printf(1, 2);
220220

221+
/*
222+
* A couple of systems offer a fast constant locale_t value representing the
223+
* "C" locale. We use that if possible, but fall back to creating a singleton
224+
* object otherwise. To check that it is available, call pg_ensure_c_locale()
225+
* and assume out of memory if it returns false.
226+
*/
227+
#ifdef LC_C_LOCALE
228+
#define PG_C_LOCALE LC_C_LOCALE
229+
#define pg_ensure_c_locale() true
230+
#else
231+
extern locale_t pg_get_c_locale(void);
232+
#define PG_C_LOCALE pg_get_c_locale()
233+
#define pg_ensure_c_locale() (PG_C_LOCALE != 0)
234+
#endif
235+
236+
#if !defined(HAVE_STRTOD_L) && !defined(WIN32)
237+
/*
238+
* POSIX doesn't define this function, but we can implement it with thread-safe
239+
* save-and-restore.
240+
*/
241+
static inline double
242+
strtod_l(const char *nptr, char **endptr, locale_t loc)
243+
{
244+
locale_t save = uselocale(loc);
245+
double result = strtod(nptr, endptr);
246+
247+
uselocale(save);
248+
return result;
249+
}
250+
#endif
251+
221252
#ifndef WIN32
222253
/*
223254
* We add a pg_ prefix as a warning that the Windows implementations have the

‎src/include/port/win32_port.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,7 @@ extern int _pglstat64(const char *name, struct stat *buf);
453453
#define isspace_l _isspace_l
454454
#define iswspace_l _iswspace_l
455455
#define strcoll_l _strcoll_l
456+
#define strtod_l _strtod_l
456457
#define strxfrm_l _strxfrm_l
457458
#define wcscoll_l _wcscoll_l
458459

‎src/interfaces/ecpg/ecpglib/connect.c

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@
1010
#include "ecpgtype.h"
1111
#include "sqlca.h"
1212

13-
#ifdef HAVE_USELOCALE
14-
locale_t ecpg_clocale = (locale_t) 0;
15-
#endif
16-
1713
static pthread_mutex_t connections_mutex = PTHREAD_MUTEX_INITIALIZER;
1814
static pthread_key_t actual_connection_key;
1915
static pthread_once_t actual_connection_key_once = PTHREAD_ONCE_INIT;
@@ -268,7 +264,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
268264
const char **conn_keywords;
269265
const char **conn_values;
270266

271-
if (sqlca == NULL)
267+
if (sqlca == NULL || !pg_ensure_c_locale())
272268
{
273269
ecpg_raise(lineno, ECPG_OUT_OF_MEMORY,
274270
ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
@@ -483,39 +479,6 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
483479
/* add connection to our list */
484480
pthread_mutex_lock(&connections_mutex);
485481

486-
/*
487-
* ... but first, make certain we have created ecpg_clocale. Rely on
488-
* holding connections_mutex to ensure this is done by only one thread.
489-
*/
490-
#ifdef HAVE_USELOCALE
491-
if (!ecpg_clocale)
492-
{
493-
ecpg_clocale = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0);
494-
if (!ecpg_clocale)
495-
{
496-
pthread_mutex_unlock(&connections_mutex);
497-
ecpg_raise(lineno, ECPG_OUT_OF_MEMORY,
498-
ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL);
499-
if (host)
500-
ecpg_free(host);
501-
if (port)
502-
ecpg_free(port);
503-
if (options)
504-
ecpg_free(options);
505-
if (realname)
506-
ecpg_free(realname);
507-
if (dbname)
508-
ecpg_free(dbname);
509-
if (conn_keywords)
510-
ecpg_free(conn_keywords);
511-
if (conn_values)
512-
ecpg_free(conn_values);
513-
free(this);
514-
return false;
515-
}
516-
}
517-
#endif
518-
519482
if (connection_name != NULL)
520483
this->name = ecpg_strdup(connection_name, lineno);
521484
else

‎src/interfaces/ecpg/ecpglib/data.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ ecpg_get_data(const PGresult *results, int act_tuple, int act_field, int lineno,
466466
pval++;
467467

468468
if (!check_special_value(pval, &dres, &scan_length))
469-
dres = strtod(pval, &scan_length);
469+
dres = strtod_l(pval, &scan_length, PG_C_LOCALE);
470470

471471
if (isarray && *scan_length == '"')
472472
scan_length++;

‎src/interfaces/ecpg/ecpglib/descriptor.c

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -475,46 +475,9 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
475475
memset(&stmt, 0, sizeof stmt);
476476
stmt.lineno = lineno;
477477

478-
/* Make sure we do NOT honor the locale for numeric input */
479-
/* since the database gives the standard decimal point */
480-
/* (see comments in execute.c) */
481-
#ifdef HAVE_USELOCALE
482-
483-
/*
484-
* To get here, the above PQnfields() test must have found nonzero
485-
* fields. One needs a connection to create such a descriptor. (EXEC
486-
* SQL SET DESCRIPTOR can populate the descriptor's "items", but it
487-
* can't change the descriptor's PQnfields().) Any successful
488-
* connection initializes ecpg_clocale.
489-
*/
490-
Assert(ecpg_clocale);
491-
stmt.oldlocale = uselocale(ecpg_clocale);
492-
#else
493-
#ifdef WIN32
494-
stmt.oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
495-
#endif
496-
stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
497-
setlocale(LC_NUMERIC, "C");
498-
#endif
499-
500478
/* desperate try to guess something sensible */
501479
stmt.connection = ecpg_get_connection(NULL);
502480
ecpg_store_result(ECPGresult, index, &stmt, &data_var);
503-
504-
#ifdef HAVE_USELOCALE
505-
if (stmt.oldlocale != (locale_t) 0)
506-
uselocale(stmt.oldlocale);
507-
#else
508-
if (stmt.oldlocale)
509-
{
510-
setlocale(LC_NUMERIC, stmt.oldlocale);
511-
ecpg_free(stmt.oldlocale);
512-
}
513-
#ifdef WIN32
514-
if (stmt.oldthreadlocale != -1)
515-
_configthreadlocale(stmt.oldthreadlocale);
516-
#endif
517-
#endif
518481
}
519482
else if (data_var.ind_type != ECPGt_NO_INDICATOR && data_var.ind_pointer != NULL)
520483

‎src/interfaces/ecpg/ecpglib/ecpglib_extern.h

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@ struct ECPGtype_information_cache
5656
enum ARRAY_TYPE isarray;
5757
};
5858

59-
#ifdef HAVE_USELOCALE
60-
extern locale_t ecpg_clocale; /* LC_NUMERIC=C */
61-
#endif
62-
6359
/* structure to store one statement */
6460
struct statement
6561
{
@@ -73,14 +69,6 @@ struct statement
7369
bool questionmarks;
7470
struct variable *inlist;
7571
struct variable *outlist;
76-
#ifdef HAVE_USELOCALE
77-
locale_t oldlocale;
78-
#else
79-
char *oldlocale;
80-
#ifdef WIN32
81-
int oldthreadlocale;
82-
#endif
83-
#endif
8472
int nparams;
8573
char **paramvalues;
8674
int *paramlengths;

‎src/interfaces/ecpg/ecpglib/execute.c

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,6 @@ free_statement(struct statement *stmt)
101101
free_variable(stmt->outlist);
102102
ecpg_free(stmt->command);
103103
ecpg_free(stmt->name);
104-
#ifndef HAVE_USELOCALE
105-
ecpg_free(stmt->oldlocale);
106-
#endif
107104
ecpg_free(stmt);
108105
}
109106

@@ -1973,43 +1970,6 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
19731970
if (stmt == NULL)
19741971
return false;
19751972

1976-
/*
1977-
* Make sure we do NOT honor the locale for numeric input/output since the
1978-
* database wants the standard decimal point. If available, use
1979-
* uselocale() for this because it's thread-safe. Windows doesn't have
1980-
* that, but it does have _configthreadlocale().
1981-
*/
1982-
#ifdef HAVE_USELOCALE
1983-
1984-
/*
1985-
* Since ecpg_init() succeeded, we have a connection. Any successful
1986-
* connection initializes ecpg_clocale.
1987-
*/
1988-
Assert(ecpg_clocale);
1989-
stmt->oldlocale = uselocale(ecpg_clocale);
1990-
if (stmt->oldlocale == (locale_t) 0)
1991-
{
1992-
ecpg_do_epilogue(stmt);
1993-
return false;
1994-
}
1995-
#else
1996-
#ifdef WIN32
1997-
stmt->oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
1998-
if (stmt->oldthreadlocale == -1)
1999-
{
2000-
ecpg_do_epilogue(stmt);
2001-
return false;
2002-
}
2003-
#endif
2004-
stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
2005-
if (stmt->oldlocale == NULL)
2006-
{
2007-
ecpg_do_epilogue(stmt);
2008-
return false;
2009-
}
2010-
setlocale(LC_NUMERIC, "C");
2011-
#endif
2012-
20131973
/*
20141974
* If statement type is ECPGst_prepnormal we are supposed to prepare the
20151975
* statement before executing them
@@ -2216,19 +2176,6 @@ ecpg_do_epilogue(struct statement *stmt)
22162176
if (stmt == NULL)
22172177
return;
22182178

2219-
#ifdef HAVE_USELOCALE
2220-
if (stmt->oldlocale != (locale_t) 0)
2221-
uselocale(stmt->oldlocale);
2222-
#else
2223-
if (stmt->oldlocale)
2224-
{
2225-
setlocale(LC_NUMERIC, stmt->oldlocale);
2226-
#ifdef WIN32
2227-
_configthreadlocale(stmt->oldthreadlocale);
2228-
#endif
2229-
}
2230-
#endif
2231-
22322179
free_statement(stmt);
22332180
}
22342181

‎src/interfaces/ecpg/pgtypeslib/dt_common.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,7 +1218,7 @@ DecodeNumber(int flen, char *str, int fmask,
12181218
return DecodeNumberField(flen, str, (fmask | DTK_DATE_M),
12191219
tmask, tm, fsec, is2digits);
12201220

1221-
*fsec = strtod(cp, &cp);
1221+
*fsec = strtod_l(cp, &cp, PG_C_LOCALE);
12221222
if (*cp != '\0')
12231223
return -1;
12241224
}
@@ -2030,7 +2030,7 @@ DecodeDateTime(char **field, int *ftype, int nf,
20302030
{
20312031
double frac;
20322032

2033-
frac = strtod(cp, &cp);
2033+
frac = strtod_l(cp, &cp, PG_C_LOCALE);
20342034
if (*cp != '\0')
20352035
return -1;
20362036
*fsec = frac * 1000000;
@@ -2054,7 +2054,7 @@ DecodeDateTime(char **field, int *ftype, int nf,
20542054
{
20552055
double time;
20562056

2057-
time = strtod(cp, &cp);
2057+
time = strtod_l(cp, &cp, PG_C_LOCALE);
20582058
if (*cp != '\0')
20592059
return -1;
20602060

‎src/interfaces/ecpg/pgtypeslib/interval.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ ParseISO8601Number(const char *str, char **endptr, int *ipart, double *fpart)
6060
if (!(isdigit((unsigned char) *str) || *str == '-' || *str == '.'))
6161
return DTERR_BAD_FORMAT;
6262
errno = 0;
63-
val = strtod(str, endptr);
63+
val = strtod_l(str, endptr, PG_C_LOCALE);
6464
/* did we not see anything that looks like a double? */
6565
if (*endptr == str || errno != 0)
6666
return DTERR_BAD_FORMAT;
@@ -455,7 +455,7 @@ DecodeInterval(char **field, int *ftype, int nf, /* int range, */
455455
else if (*cp == '.')
456456
{
457457
errno = 0;
458-
fval = strtod(cp, &cp);
458+
fval = strtod_l(cp, &cp, PG_C_LOCALE);
459459
if (*cp != '\0' || errno != 0)
460460
return DTERR_BAD_FORMAT;
461461

‎src/interfaces/ecpg/pgtypeslib/numeric.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1455,7 +1455,7 @@ numericvar_to_double(numeric *var, double *dp)
14551455
* strtod does not reset errno to 0 in case of success.
14561456
*/
14571457
errno = 0;
1458-
val = strtod(tmp, &endptr);
1458+
val = strtod_l(tmp, &endptr, PG_C_LOCALE);
14591459
if (errno == ERANGE)
14601460
{
14611461
free(tmp);

‎src/port/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ OBJS = \
4141
bsearch_arg.o \
4242
chklocale.o \
4343
inet_net_ntop.o \
44+
locale.o \
4445
noblock.o \
4546
path.o \
4647
pg_bitutils.o \

0 commit comments

Comments
 (0)