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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3162 bytes --]

On 06/03/2016 03:02 AM, Markus Armbruster wrote:

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

How about this alternative:

Finite values remain numbers:
"number":1

But non-finite values are output as strings, so that our output is
always valid JSON - the recipient may not be expecting a string in place
of a number, but at least should be able to parse the output rather than
choking hard.
"number":"nan"

The return value -1 then indicates that a stringized replacement was
used, so that any later patch can use a strict flag on whether to allow
the replacement output or assert.

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

I may still try to tackle fixing the QMP parser to accept NaN and
infinity on input (since it's hand-written, we at least have control
over that) - it will certainly be easier than getting libvirt to parse
non-finite numbers (libvirt uses libyajl, and my emails to the yajl
mailing list have gone unanswered, making me think the project is not
very vibrant and thus not very patchable).  But with my proposal of
producing a stringized non-finite value, we at least convert lexical
into semantic errors, which I agree with your assessment is a nicer way
of dealing with it.

Of course, a policy change of outputting stringized non-finite numbers
should be separate from refactoring patches that just move functions around.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]