Discussion on compile flag enabled by default (ENABLE_LOCALES)

[Zykino]

So I just submitted an issue to the AWS SDK because I got an issue with them see the stack overflow quesion for full details.
TL;DR AWS SDK copied cJSON in their source and compiled without the ENABLE_LOCALES defined even if in your compilation step it is ON by default.

I am here more to discuss about this than to "report a bug" in the traditional sense.

Environment ON by default

Once I found my issue I found #202 which made me understood why ENABLE_LOCALES even exist. But as all programmers are humans and all human don't read the documentation makes errors, I think my problem is an solicitation to never have compile variable ON by default. I know that DISABLE_LOCALES=ON sound very strange, but we may find a variable name that permit less error on simple "copy/past make my own installation". Something like DESACTIVATE_LOCALES or COMPILE_WITHOUT_LOCALES I don't know.

Code

My first fix was to increment the input buffer offset by i instead of by the result of strtod aka (size_t)(after_end - number_c_string). I don't really know what is the stance of cJSON on printing errors messages (or using asserts) but to me something like this could be prevented by an assert in the lines of this pseudocode: assert(i, (size_t)(after_end - number_c_string), "cJSON was compiled with locales disabled but we have a mismatch between the number of character we detect for the number <show number's name and value> and the number of characters parsed as number. This is probably is caused by the fact you are not using a computer with English locales while having the compilation variable ENABLE_LOCALES unsetted.");.

On or the other of both of this approach would have saved me quite some time in looking for the issue.

By the way the man strtod example refers to man strtol for the good usage of this functions and the example check errno to potentials errors. I am not sure if this is needed since cJSON read the number value before, but that can't hurt.
Part of the man strtol example:

errno = 0;    /* To distinguish success/failure after call */
val = strtol(str, &endptr, base);

/* Check for various possible errors */

if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
       || (errno != 0 && val == 0)) {
   perror("strtol");
   exit(EXIT_FAILURE);
}

Return false (with a log about system capability ?) instead of in cJSON.

Edit: I almost forgot to say the other code option I have thought of that is asking for either a culture invariant version of strtod or a version where we can pass the decimal point as a parameter. The first one would mean that protocol (like JSON) could work whatever the current culture is, while the second one is a more flexible option and more in line with what propose higher level languages/frameworks.
I don't know where to propose that kind of change (libc ? how to contact them ?).

[danpla]

The library is currently broken for anyone who installed it by copying the files rather than via CMake. If the current locale uses a comma as a decimal separator, cJSON will:

1. Silently trim numbers on reading (Cognito STS refused to be given to host application because bad compilation of cJSON aws/aws-sdk-cpp#1474)
2. Produce invalid JSON on writing (cJSON_Print generates invalid JSOn when locale uses comma as decimal point #699)

The simple fix is to add something like this in the C file:

#ifndef ENABLE_LOCALES
#define ENABLE_LOCALES 1
#endif