From 00de20e9d0b855341c6ea5dd2be4c2c14f7a4cc8 Mon Sep 17 00:00:00 2001 From: Petri Lehtinen Date: Thu, 7 Mar 2024 21:02:55 +0200 Subject: [PATCH] Use sprintf() to determine locale's decimal point This should fix thread safety of encoding and decoding, since localeconv() is not tread safe after all. --- CMakeLists.txt | 13 ------------ android/jansson_config.h | 4 ---- cmake/jansson_config.h.cmake | 3 --- configure.ac | 10 ++-------- doc/threadsafety.rst | 22 --------------------- src/jansson_config.h.in | 4 ---- src/strconv.c | 38 ++++++++++++------------------------ test/bin/json_process.c | 4 ---- test/suites/api/util.h | 3 --- 9 files changed, 15 insertions(+), 86 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ed33e3c4..550c0d5c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -216,19 +216,6 @@ if (NOT DEFINED JSON_INT_T) endif () -# If locale.h and localeconv() are available, define to 1, otherwise to 0. -check_include_files (locale.h HAVE_LOCALE_H) -check_function_exists (localeconv HAVE_LOCALECONV) - -if (HAVE_LOCALECONV AND HAVE_LOCALE_H) - set (JSON_HAVE_LOCALECONV 1) -else () - set (JSON_HAVE_LOCALECONV 0) -endif() - -# check if we have setlocale -check_function_exists(setlocale HAVE_SETLOCALE) - # Check what the inline keyword is. # Note that the original JSON_INLINE was always set to just 'inline', so this goes further. check_function_keywords("inline") diff --git a/android/jansson_config.h b/android/jansson_config.h index 618a0da7..448010a4 100644 --- a/android/jansson_config.h +++ b/android/jansson_config.h @@ -32,10 +32,6 @@ otherwise to 0. */ #define JSON_INTEGER_IS_LONG_LONG 1 -/* If locale.h and localeconv() are available, define to 1, - otherwise to 0. */ -#define JSON_HAVE_LOCALECONV 0 - /* Maximum recursion depth for parsing JSON input. This limits the depth of e.g. array-within-array constructions. */ #define JSON_PARSER_MAX_DEPTH 2048 diff --git a/cmake/jansson_config.h.cmake b/cmake/jansson_config.h.cmake index 2f248cbc..542e57b4 100644 --- a/cmake/jansson_config.h.cmake +++ b/cmake/jansson_config.h.cmake @@ -56,9 +56,6 @@ #define JSON_INTEGER_FORMAT @JSON_INTEGER_FORMAT@ -/* If locale.h and localeconv() are available, define to 1, otherwise to 0. */ -#define JSON_HAVE_LOCALECONV @JSON_HAVE_LOCALECONV@ - /* If __atomic builtins are available they will be used to manage reference counts of json_t. */ #define JSON_HAVE_ATOMIC_BUILTINS @JSON_HAVE_ATOMIC_BUILTINS@ diff --git a/configure.ac b/configure.ac index f022eb72..9ff76b45 100644 --- a/configure.ac +++ b/configure.ac @@ -16,7 +16,7 @@ AM_CONDITIONAL([GCC], [test x$GCC = xyes]) # Checks for libraries. # Checks for header files. -AC_CHECK_HEADERS([endian.h fcntl.h locale.h sched.h unistd.h sys/param.h sys/stat.h sys/time.h sys/types.h]) +AC_CHECK_HEADERS([endian.h fcntl.h sched.h unistd.h sys/param.h sys/stat.h sys/time.h sys/types.h]) # Checks for typedefs, structures, and compiler characteristics. AC_TYPE_INT32_T @@ -34,7 +34,7 @@ esac AC_SUBST([json_inline]) # Checks for library functions. -AC_CHECK_FUNCS([close getpid gettimeofday localeconv open read sched_yield strtoll]) +AC_CHECK_FUNCS([close getpid gettimeofday open read sched_yield strtoll]) AC_MSG_CHECKING([for gcc __sync builtins]) have_sync_builtins=no @@ -74,12 +74,6 @@ case "$ac_cv_type_long_long_int$ac_cv_func_strtoll" in esac AC_SUBST([json_have_long_long]) -case "$ac_cv_header_locale_h$ac_cv_func_localeconv" in - yesyes) json_have_localeconv=1;; - *) json_have_localeconv=0;; -esac -AC_SUBST([json_have_localeconv]) - # Features AC_ARG_ENABLE([urandom], [AS_HELP_STRING([--disable-urandom], diff --git a/doc/threadsafety.rst b/doc/threadsafety.rst index 0eebb29a..0832bdce 100644 --- a/doc/threadsafety.rst +++ b/doc/threadsafety.rst @@ -58,25 +58,3 @@ Memory allocation functions Memory allocation functions should be set at most once, and only on program startup. See :ref:`apiref-custom-memory-allocation`. - - -Locale -====== - -Jansson works fine under any locale. - -However, if the host program is multithreaded and uses ``setlocale()`` -to switch the locale in one thread while Jansson is currently encoding -or decoding JSON data in another thread, the result may be wrong or -the program may even crash. - -Jansson uses locale specific functions for certain string conversions -in the encoder and decoder, and then converts the locale specific -values to/from the JSON representation. This fails if the locale -changes between the string conversion and the locale-to-JSON -conversion. This can only happen in multithreaded programs that use -``setlocale()``, because ``setlocale()`` switches the locale for all -running threads, not only the thread that calls ``setlocale()``. - -If your program uses ``setlocale()`` as described above, consider -using the thread-safe ``uselocale()`` instead. diff --git a/src/jansson_config.h.in b/src/jansson_config.h.in index fe692ab4..791f60d0 100644 --- a/src/jansson_config.h.in +++ b/src/jansson_config.h.in @@ -32,10 +32,6 @@ otherwise to 0. */ #define JSON_INTEGER_IS_LONG_LONG @json_have_long_long@ -/* If locale.h and localeconv() are available, define to 1, - otherwise to 0. */ -#define JSON_HAVE_LOCALECONV @json_have_localeconv@ - /* If __atomic builtins are available they will be used to manage reference counts of json_t. */ #define JSON_HAVE_ATOMIC_BUILTINS @json_have_atomic_builtins@ diff --git a/src/strconv.c b/src/strconv.c index c6f4fd16..f32680de 100644 --- a/src/strconv.c +++ b/src/strconv.c @@ -11,57 +11,47 @@ #include #endif -#if JSON_HAVE_LOCALECONV -#include - -/* - - This code assumes that the decimal separator is exactly one - character. - - - If setlocale() is called by another thread between the call to - localeconv() and the call to sprintf() or strtod(), the result may - be wrong. setlocale() is not thread-safe and should not be used - this way. Multi-threaded programs should use uselocale() instead. -*/ +static char get_decimal_point() { + char buf[3]; + snprintf(&buf, 3, "%#.0f", 1.0); // "1." in the current locale + return buf[1]; +} static void to_locale(strbuffer_t *strbuffer) { - const char *point; + char point; char *pos; - point = localeconv()->decimal_point; - if (*point == '.') { + point = get_decimal_point(); + if (point == '.') { /* No conversion needed */ return; } pos = strchr(strbuffer->value, '.'); if (pos) - *pos = *point; + *pos = point; } static void from_locale(char *buffer) { - const char *point; + char point; char *pos; - point = localeconv()->decimal_point; - if (*point == '.') { + point = get_decimal_point(); + if (point == '.') { /* No conversion needed */ return; } - pos = strchr(buffer, *point); + pos = strchr(buffer, point); if (pos) *pos = '.'; } -#endif int jsonp_strtod(strbuffer_t *strbuffer, double *out) { double value; char *end; -#if JSON_HAVE_LOCALECONV to_locale(strbuffer); -#endif errno = 0; value = strtod(strbuffer->value, &end); @@ -92,9 +82,7 @@ int jsonp_dtostr(char *buffer, size_t size, double value, int precision) { if (length >= size) return -1; -#if JSON_HAVE_LOCALECONV from_locale(buffer); -#endif /* Make sure there's a dot or 'e' in the output. Otherwise a real is converted to an integer when decoding */ diff --git a/test/bin/json_process.c b/test/bin/json_process.c index fc98543f..7b89e8df 100644 --- a/test/bin/json_process.c +++ b/test/bin/json_process.c @@ -333,10 +333,6 @@ int main(int argc, char *argv[]) { int i; char *test_path = NULL; -#ifdef HAVE_SETLOCALE - setlocale(LC_ALL, ""); -#endif - if (argc < 2) { goto usage; } diff --git a/test/suites/api/util.h b/test/suites/api/util.h index d964c493..9c08a9f3 100644 --- a/test/suites/api/util.h +++ b/test/suites/api/util.h @@ -83,9 +83,6 @@ static void run_tests(); int main() { -#ifdef HAVE_SETLOCALE - setlocale(LC_ALL, ""); -#endif run_tests(); return 0; }