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

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 06/02/2016 09:02 AM, Markus Armbruster wrote:
>> 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.
>
>> 
>> 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.
>
> I'm actually thinking of modifying this, given the recent thread
> pointing out that libvirt chokes hard on JSON extensions:
>
> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg04398.html
>
> That is, for symmetry with qstring_append_json_string(), I'm thinking of
> changing NaN to 0 and Inf to DBL_MAX, and always outputting a finite
> value, in addition to returning -1 to inform the caller that a
> substitution was made, so that the output is always strict JSON.

Mapping infinities to DBL_MIN and DBL_MAX is debatable, but mapping NaN
to zero is outright wrong.

If we decide QMP should stick to JSON here and avoid non-finite numbers,
we need to treat an attempt to emit a non-finite number as a bug:
assert(isfinite(...)).  Making sure nothing ever attempts to emit such
numbers will be tedious.

If we decide QMP should remain as it is, we need to document non-finite
numbers among its JSON extensions.  We should also fix our QMP parsers
to accept non-finite numbers then.  Including the one in libvirt.
Attempts to emit non-finite numbers then *may* be bugs.  Really no
different than finite numbers outside their intended range, such as a
negative size.  Catching these bugs is of course also tedious.  The
difference is that they manifest in QMP as semantic instead of lexical
errors.  Lexical errors are the worst to handle gracefully.