Re: [Qemu-devel] [PATCH v4 17/28] qapi: Factor out JSON number formatting

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> Pull out a new qstring_append_json_number() helper, so that all
> JSON output producers can use a consistent style for printing
> floating point without duplicating code (since we are doing more
> data massaging than a simple printf format can handle).  (For
> now, there is only one client, but later patches will use it.)
>
> Adding a return value will let callers that care diagnose the
> situation where an attempt was made to output invalid JSON (which
> does not specify Infinity or NaN). None of the current callers
> care, but a future patch wants to make it possible to turn this
> situation into an error.
>
> Signed-off-by: Eric Blake <address@hidden>
[...]
> @@ -303,3 +282,39 @@ int qstring_append_json_string(QString *qstring, const 
> char *str)
>      qstring_append(qstring, "\"");
>      return result;
>  }
> +
> +/**
> + * Append a JSON representation of @number to @qstring.
> + *
> + * Returns -1 if the added text is not strict JSON, or 0 if the number
> + * was finite.
> + */

Suggest:

 * Return 0 if the number is finite, as required by RFC 7159, else -1.

The return value makes some sense only for symmetry with
qstring_append_json_string().  Without that, I'd ask you to keep this
function simple.  Callers could just as easily test isfinite()
themselves.

> +int qstring_append_json_number(QString *qstring, double number)
> +{
> +    char buffer[1024];
> +    int len;
> +
> +    /* FIXME: snprintf() is locale dependent; but JSON requires
> +     * numbers to be formatted as if in the C locale. Dependence
> +     * on C locale is a pervasive issue in QEMU. */
> +    /* FIXME: This risks printing Inf or NaN, which are not valid
> +     * JSON values. */
> +    /* FIXME: the default precision of 6 for %f often causes
> +     * rounding errors; we should be using DBL_DECIMAL_DIG (17),
> +     * and only rounding to a shorter number if the result would
> +     * still produce the same floating point value.  */
> +    len = snprintf(buffer, sizeof(buffer), "%f", number);
> +    assert(len > 0 && len < sizeof(buffer));
> +    while (len > 0 && buffer[len - 1] == '0') {
> +        len--;
> +    }
> +
> +    if (len && buffer[len - 1] == '.') {
> +        buffer[len - 1] = 0;
> +    } else {
> +        buffer[len] = 0;
> +    }
> +
> +    qstring_append(qstring, buffer);
> +    return isfinite(number) ? 0 : -1;
> +}