Skip to content

Commit

Permalink
Use sprintf() to determine locale's decimal point
Browse files Browse the repository at this point in the history
This should fix thread safety of encoding and decoding, since
localeconv() is not tread safe after all.
  • Loading branch information
akheron committed Mar 7, 2024
1 parent 7e04530 commit eba3139
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 64 deletions.
13 changes: 0 additions & 13 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
4 changes: 0 additions & 4 deletions android/jansson_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions cmake/jansson_config.h.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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@
Expand Down
10 changes: 2 additions & 8 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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],
Expand Down
4 changes: 0 additions & 4 deletions src/jansson_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -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@
Expand Down
38 changes: 13 additions & 25 deletions src/strconv.c
Original file line number Diff line number Diff line change
Expand Up @@ -11,57 +11,47 @@
#include <jansson_private_config.h>
#endif

#if JSON_HAVE_LOCALECONV
#include <locale.h>

/*
- 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];
sprintf(buf, "%#.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);
Expand Down Expand Up @@ -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 */
Expand Down
4 changes: 0 additions & 4 deletions test/bin/json_process.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
3 changes: 0 additions & 3 deletions test/suites/api/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@
static void run_tests();

int main() {
#ifdef HAVE_SETLOCALE
setlocale(LC_ALL, "");
#endif
run_tests();
return 0;
}
Expand Down

0 comments on commit eba3139

Please sign in to comment.