From 749fefc0c4ef686bed3fbf7b7dd436582aa7034d Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Sat, 8 Apr 2017 02:41:36 +0200 Subject: [PATCH 1/5] Make parse_number locale independent --- cJSON.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/cJSON.c b/cJSON.c index 975bcab..d8d0f49 100644 --- a/cJSON.c +++ b/cJSON.c @@ -31,6 +31,7 @@ #include #include #include +#include #pragma GCC visibility pop #include "cJSON.h" @@ -177,19 +178,63 @@ CJSON_PUBLIC(void) cJSON_Delete(cJSON *c) } } +/* get the decimal point character of the current locale */ +static unsigned char get_decimal_point(void) +{ + struct lconv *lconv = localeconv(); + return (unsigned char) lconv->decimal_point[0]; +} + /* Parse the input text to generate a number, and populate the result into item. */ static const unsigned char *parse_number(cJSON * const item, const unsigned char * const input) { double number = 0; unsigned char *after_end = NULL; + unsigned char number_c_string[64]; + unsigned char decimal_point = get_decimal_point(); + size_t i = 0; if (input == NULL) { return NULL; } - number = strtod((const char*)input, (char**)&after_end); - if (input == after_end) + /* copy the number into a temporary buffer and replace '.' with the decimal point + * of the current locale (for strtod) */ + for (i = 0; (i < (sizeof(number_c_string) - 1)) && (input[i] != '\0'); i++) + { + switch (input[i]) + { + case '0': + case '1': + case '2': + case '3': + case '4': + case '5': + case '6': + case '7': + case '8': + case '9': + case '+': + case '-': + case 'e': + case 'E': + number_c_string[i] = input[i]; + break; + + case '.': + number_c_string[i] = decimal_point; + break; + + default: + goto loop_end; + } + } +loop_end: + number_c_string[i] = '\0'; + + number = strtod((const char*)number_c_string, (char**)&after_end); + if (number_c_string == after_end) { return NULL; /* parse_error */ } @@ -212,7 +257,7 @@ static const unsigned char *parse_number(cJSON * const item, const unsigned char item->type = cJSON_Number; - return after_end; + return input + (after_end - number_c_string); } /* don't ask me, but the original cJSON_SetNumberValue returns an integer or double */ From 71b96afc27cc408952879e24f492e542b866826c Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Sat, 8 Apr 2017 02:46:24 +0200 Subject: [PATCH 2/5] print_number: Fix comment (missing word 'zeroes') --- cJSON.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cJSON.c b/cJSON.c index d8d0f49..0297a14 100644 --- a/cJSON.c +++ b/cJSON.c @@ -417,7 +417,7 @@ static cJSON_bool print_number(const cJSON * const item, printbuffer * const out unsigned char *output_pointer = NULL; double d = item->valuedouble; int length = 0; - cJSON_bool trim_zeroes = true; /* should at the end be removed? */ + cJSON_bool trim_zeroes = true; /* should zeroes at the end be removed? */ if (output_buffer == NULL) { From c08f7e1d290aebde2df8cd5e87f4f3ebf64f7318 Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Sat, 8 Apr 2017 03:07:13 +0200 Subject: [PATCH 3/5] print_number: Make locale independent This first prints the number into a temporary buffer and then copies the number to the output. A positive side effect is that cJSON no longer reserves more space for the number in the output than is necessary. --- cJSON.c | 81 +++++++++++++++++++++++++------------------- tests/print_number.c | 16 +++------ 2 files changed, 51 insertions(+), 46 deletions(-) diff --git a/cJSON.c b/cJSON.c index 0297a14..581b2f6 100644 --- a/cJSON.c +++ b/cJSON.c @@ -381,34 +381,24 @@ static void update_offset(printbuffer * const buffer) } /* Removes trailing zeroes from the end of a printed number */ -static cJSON_bool trim_trailing_zeroes(printbuffer * const buffer) +static int trim_trailing_zeroes(const unsigned char * const number, int length, const unsigned char decimal_point) { - size_t offset = 0; - unsigned char *content = NULL; - - if ((buffer == NULL) || (buffer->buffer == NULL) || (buffer->offset < 1)) + if ((number == NULL) || (length <= 0)) { - return false; + return -1; } - offset = buffer->offset - 1; - content = buffer->buffer; - - while ((offset > 0) && (content[offset] == '0')) + while ((length > 0) && (number[length - 1] == '0')) { - offset--; + length--; } - if ((offset > 0) && (content[offset] == '.')) + if ((length > 0) && (number[length - 1] == decimal_point)) { - offset--; + /* remove trailing decimal_point */ + length--; } - offset++; - content[offset] = '\0'; - - buffer->offset = offset; - - return true; + return length; } /* Render the number nicely from the given item into a string. */ @@ -417,54 +407,75 @@ static cJSON_bool print_number(const cJSON * const item, printbuffer * const out unsigned char *output_pointer = NULL; double d = item->valuedouble; int length = 0; + size_t i = 0; cJSON_bool trim_zeroes = true; /* should zeroes at the end be removed? */ + unsigned char number_buffer[64]; /* temporary buffer to print the number into */ + unsigned char decimal_point = get_decimal_point(); if (output_buffer == NULL) { return false; } - /* This is a nice tradeoff. */ - output_pointer = ensure(output_buffer, 64, hooks); - if (output_pointer == NULL) - { - return false; - } - /* This checks for NaN and Infinity */ if ((d * 0) != 0) { - length = sprintf((char*)output_pointer, "null"); + length = sprintf((char*)number_buffer, "null"); } else if ((fabs(floor(d) - d) <= DBL_EPSILON) && (fabs(d) < 1.0e60)) { /* integer */ - length = sprintf((char*)output_pointer, "%.0f", d); + length = sprintf((char*)number_buffer, "%.0f", d); trim_zeroes = false; /* don't remove zeroes for "big integers" */ } else if ((fabs(d) < 1.0e-6) || (fabs(d) > 1.0e9)) { - length = sprintf((char*)output_pointer, "%e", d); + length = sprintf((char*)number_buffer, "%e", d); trim_zeroes = false; /* don't remove zeroes in engineering notation */ } else { - length = sprintf((char*)output_pointer, "%f", d); + length = sprintf((char*)number_buffer, "%f", d); } - /* sprintf failed */ - if (length < 0) + /* sprintf failed or buffer overrun occured */ + if ((length < 0) || (length > (int)(sizeof(number_buffer) - 1))) { return false; } - output_buffer->offset += (size_t)length; - if (trim_zeroes) { - return trim_trailing_zeroes(output_buffer); + length = trim_trailing_zeroes(number_buffer, length, decimal_point); + if (length <= 0) + { + return false; + } } + /* reserve appropriate space in the output */ + output_pointer = ensure(output_buffer, (size_t)length, hooks); + if (output_pointer == NULL) + { + return false; + } + + /* copy the printed number to the output and replace locale + * dependent decimal point with '.' */ + for (i = 0; i < ((size_t)length); i++) + { + if (number_buffer[i] == decimal_point) + { + output_pointer[i] = '.'; + continue; + } + + output_pointer[i] = number_buffer[i]; + } + output_pointer[i] = '\0'; + + output_buffer->offset += (size_t)length; + return true; } diff --git a/tests/print_number.c b/tests/print_number.c index 0908f88..16f08c2 100644 --- a/tests/print_number.c +++ b/tests/print_number.c @@ -89,17 +89,11 @@ static void print_number_should_print_non_number(void) static void trim_trailing_zeroes_should_trim_trailing_zeroes(void) { - printbuffer buffer; - unsigned char number[100]; - buffer.length = sizeof(number); - buffer.buffer = number; - - strcpy((char*)number, "10.00"); - buffer.offset = sizeof("10.00") - 1; - TEST_ASSERT_TRUE(trim_trailing_zeroes(&buffer)); - TEST_ASSERT_EQUAL_UINT8('\0', buffer.buffer[buffer.offset]); - TEST_ASSERT_EQUAL_STRING("10", number); - TEST_ASSERT_EQUAL_UINT(sizeof("10") - 1, buffer.offset); + TEST_ASSERT_EQUAL_INT(2, trim_trailing_zeroes((const unsigned char*)"10.00", (int)(sizeof("10.00") - 1), '.')); + TEST_ASSERT_EQUAL_INT(0, trim_trailing_zeroes((const unsigned char*)".00", (int)(sizeof(".00") - 1), '.')); + TEST_ASSERT_EQUAL_INT(0, trim_trailing_zeroes((const unsigned char*)"00", (int)(sizeof("00") - 1), '.')); + TEST_ASSERT_EQUAL_INT(-1, trim_trailing_zeroes(NULL, 10, '.')); + TEST_ASSERT_EQUAL_INT(-1, trim_trailing_zeroes((const unsigned char*)"", 0, '.')); } int main(void) From 65541b900c740e1d527cd4f1935eec3740d4d95a Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Sat, 8 Apr 2017 03:42:44 +0200 Subject: [PATCH 4/5] Update space requirements of cJSON_PrintPreallocated --- cJSON.h | 2 +- test.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cJSON.h b/cJSON.h index d3f492c..c2ef2fa 100644 --- a/cJSON.h +++ b/cJSON.h @@ -133,7 +133,7 @@ CJSON_PUBLIC(char *) cJSON_PrintUnformatted(const cJSON *item); /* Render a cJSON entity to text using a buffered strategy. prebuffer is a guess at the final size. guessing well reduces reallocation. fmt=0 gives unformatted, =1 gives formatted */ CJSON_PUBLIC(char *) cJSON_PrintBuffered(const cJSON *item, int prebuffer, cJSON_bool fmt); /* Render a cJSON entity to text using a buffer already allocated in memory with given length. Returns 1 on success and 0 on failure. */ -/* NOTE: If you are printing numbers, the buffer hat to be 63 bytes bigger then the printed JSON (worst case) */ +/* NOTE: cJSON is not always 100% accurate in estimating how much memory it will use, so to be safe allocate 5 bytes more than you actually need */ CJSON_PUBLIC(cJSON_bool) cJSON_PrintPreallocated(cJSON *item, char *buffer, const int length, const cJSON_bool format); /* Delete a cJSON entity and all subentities. */ CJSON_PUBLIC(void) cJSON_Delete(cJSON *c); diff --git a/test.c b/test.c index 34be4d7..e902c71 100644 --- a/test.c +++ b/test.c @@ -53,8 +53,8 @@ static int print_preallocated(cJSON *root) out = cJSON_Print(root); /* create buffer to succeed */ - /* the extra 64 bytes are in case a floating point value is printed */ - len = strlen(out) + 64; + /* the extra 5 bytes are because of inaccuracies when reserving memory */ + len = strlen(out) + 5; buf = (char*)malloc(len); if (buf == NULL) { From 3efef58c323e17e61de853d6418a89193b5e9e8e Mon Sep 17 00:00:00 2001 From: Max Bruckner Date: Sat, 8 Apr 2017 03:50:22 +0200 Subject: [PATCH 5/5] README: Add setlocale to caveats --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0c3e512..2938178 100644 --- a/README.md +++ b/README.md @@ -392,8 +392,9 @@ The maximum length of a floating point literal that cJSON supports is currently In general cJSON is **not thread safe**. However it is thread safe under the following conditions: -* You don't use `cJSON_GetErrorPtr` (you can use the `return_parse_end` parameter of `cJSON_ParseWithOpts` instead) -* You only ever call `cJSON_InitHooks` before using cJSON in any threads. +* `cJSON_GetErrorPtr` is never used (the `return_parse_end` parameter of `cJSON_ParseWithOpts` can be used instead) +* `cJSON_InitHooks` is only ever called before using cJSON in any threads. +* `setlocale` is never called before all calls to cJSON functions have returned. # Enjoy cJSON!