Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
samblg opened this issue Mar 7, 2024 · 0 comments

Comments

@samblg
Copy link

samblg commented Mar 7, 2024

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 #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 #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.

@samblg samblg changed the title Regression on double precision in print_number() when a clang compile warning Regression on double precision in print_number() when fix a clang compile warning Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant