DaveGamble / cJSON

Ultralightweight JSON parser in ANSI C
MIT License
10.68k stars 3.21k forks source link

Regression on double precision in print_number() when fix a clang compile warning #838

Open samblg opened 6 months ago

samblg commented 6 months ago

When recently we integrated with the latest release of cJSON (v1.7.17), we noticed there's an regression with cJSON_Print() function when output the double numbers, the precesion is no longer the original one (compared to earlier version before https://github.com/DaveGamble/cJSON/pull/368/files#diff-65da902479919eac5c3df0d01d76628b7df2392b121fb4f746c40b42daed525cR516).

We usually store std::numeric_limits::min and std::numeric_limits::max when a double value was not set and store it into a json eventually.

For instance, when the added double is 1.7976931348623157e+308, it looks good in memory, however, when print the json out, it turned into 1.79769313486232e+308, there's a precision loss after print out compared to what stored in memory.

This should be relative to https://github.com/DaveGamble/cJSON/pull/368, which intended to fix a clang compile warning issue, it changed:

/* Render the number nicely from the given item into a string. */
static cJSON_bool print_number(const cJSON * const item, printbuffer * const output_buffer)
{
    unsigned char *output_pointer = NULL;
    double d = item->valuedouble;
    int length = 0;
    size_t i = 0;
    unsigned char number_buffer[26] = {0}; /* temporary buffer to print the number into */
    unsigned char decimal_point = get_decimal_point();
    double test = 0.0;
    if (output_buffer == NULL)
    {
        return false;
    }

    /* This checks for NaN and Infinity */
    if ((d * 0) != 0)
    {
        length = sprintf((char*)number_buffer, "null");
    }
    else
    {
        /* Try 15 decimal places of precision to avoid nonsignificant nonzero digits */
        length = sprintf((char*)number_buffer, "%1.15g", d);

        /* Check whether the original double can be recovered */
        if ((sscanf((char*)number_buffer, "%lg", &test) != 1) || ((double)test != d))
        {
            /* If not, print with 17 decimal places of precision */
            length = sprintf((char*)number_buffer, "%1.17g", d);
        }
    }

into:

/* Render the number nicely from the given item into a string. */
static cJSON_bool print_number(const cJSON * const item, printbuffer * const output_buffer)
{
    unsigned char *output_pointer = NULL;
    double d = item->valuedouble;
    int length = 0;
    size_t i = 0;
    unsigned char number_buffer[26] = {0}; /* temporary buffer to print the number into */
    unsigned char decimal_point = get_decimal_point();
    double test = 0.0;
    if (output_buffer == NULL)
    {
        return false;
    }

    /* This checks for NaN and Infinity */
    if (!compare_double(d * 0, 0))
    {
        length = sprintf((char*)number_buffer, "null");
    }
    else
    {
        /* Try 15 decimal places of precision to avoid nonsignificant nonzero digits */
        length = sprintf((char*)number_buffer, "%1.15g", d);

        /* Check whether the original double can be recovered */
        if ((sscanf((char*)number_buffer, "%lg", &test) != 1) || !compare_double((double)test, d))
        {
            /* If not, print with 17 decimal places of precision */
            length = sprintf((char*)number_buffer, "%1.17g", d);
        }
    }

So, by introducing in compare_double() function for this line of code:

if ((sscanf((char*)number_buffer, "%lg", &test) != 1) || ((double)test != d))

For the number I posted above will no longer goes into the subsequent code block:

{
            /* If not, print with 17 decimal places of precision */
            length = sprintf((char*)number_buffer, "%1.17g", d);
        }

This makes the ouput string inconsistent with what stored/added in memory at runtime.

We are using cJSON to store graphics data, for instance the world bounding box (matrix), this result in the different precesion of the double and caused the inaccurate graphcis data.

I noticed this was introduced in by a PR that tries to fix a compiling error. This regression (or call it a defect) has significant side-effect on the json output for the use case that requires the precised and consistent output as what user inputs.

The problematic PR is https://github.com/DaveGamble/cJSON/pull/368

I believe this is a regression and it has very bad impact on the use case that values the double precision. A fix would be highly required and appreciated.