[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 17/28] qapi: Factor out JSON number formattin
Re: [Qemu-devel] [PATCH v4 17/28] qapi: Factor out JSON number formatting
Mon, 13 Jun 2016 10:22:38 +0200
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)
Eric Blake <address@hidden> writes:
> On 06/03/2016 03:02 AM, Markus Armbruster wrote:
>>>> * 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()
>>> I'm actually thinking of modifying this, given the recent thread
>>> pointing out that libvirt chokes hard on JSON extensions:
>>> 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:
> 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.
> 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.
I dislike this a lot less than mapping non-finite numbers to finite
I still dislike it, because it defeats fitting a schema to QMP: instead
of the true JSON type "number", we'd need the sum type of "number" and
"string", which this really isn't: only a few special strings are valid,
and they're not actually strings. If the schema language can do sum
types, we'd even be stuck with their common super-type.
The most practical solution isn't always a likable one, though.
>> 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)
Making json-lexer.c recognize infinities and NaNs in strtod() syntax
shouldn't be hard. I'd omit nan(n-char-sequence-opt), because its
semantics are implementation defined. I'd also omit all spellings other
than inf and nan. That leaves inf, +inf, -inf, nan, +nan, -nan.
> - 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).
Nobody likes to carry downstream patches, but an unresponsive upstream
may leave you no choice.
> 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.