qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v5 01/28] qapi: Document type-safety considerati


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v5 01/28] qapi: Document type-safety considerations
Date: Thu, 26 Mar 2015 10:09:20 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 03/25/2015 02:11 PM, Eric Blake wrote:
>
>>> The QObject types are QTYPE_NONE, QTYPE_QINT, QTYPE_QSTRING,
>>> QTYPE_QDICT, QTYPE_QLIST, QTYPE_QFLOAT, QTYPE_QBOOL, QTYPE_QERROR.
>>>
>>> The connections JSON string - QTYPE_QSTRING, JSON object - QTYPE_QDICT,
>>> JSON array - QTYPE_QLIST and JSON boolean - QTYPE_QBOOL are obvious
>>> enough.
>>>
>>> If I remember correctly, our JSON parser chokes on the JSON keyword
>>> null.
>
> Yep, that is sadly still the case [1]:
>
> $ qemu-kvm -qmp stdio -nodefaults
> {"QMP": {"version": {"qemu": {"micro": 90, "minor": 2, "major": 2},
> "package": " (qemu-2.3.0-0.1.rc0.fc21)"}, "capabilities": []}}
> {"execute":"qmp_capabilities","id":null}
> {"error": {"class": "GenericError", "desc": "Invalid JSON syntax"}}
>
> Patch 17 adds support for it to qapi schemas, but not to our JSON
> parser.  I guess that's yet another task to be solved before we turn on
> argument defaults and introspection (as returning 'null' would be a
> logical solution for introspecting the default value of an optional
> string argument.  On the other hand, we could argue that using 'null' in
> output of introspection still doesn't require the parser to accept
> 'null' on input;  the output direction of introspection is different
> than the input direction of parsing a 'null' in user commands).

Yes.

Our parser choking on null is simply a bug that happens not to matter
with our current usage.

>>>
>>> That leaves just JSON numbers - QTYPE_QINT or QTYPE_QFLOAT.  Can an
>>> anonymous union have a separate case for each of the two?
>
> Yes.  Don't know why we'd want it, but the code currently handles it.
> That is, this compiles just fine:
>
> { 'alternate': 'Foo', 'data': { 'a': 'int', 'b': 'number' } }
> { 'command': 'bar', 'data': { 'value': 'Foo' } }
>
> allowing:
>  {"execute":"bar", "arguments":{ "value":1 } }
>  {"execute":"bar", "arguments":{ "value":1.0 } }
> as operating on uint64_t vs. double (in practice, since '1' is also
> valid input as a number, a double is sufficient without needing the
> alternative of a uint64_t, unless you are simultaneously worrying about
> precise integral values larger than 2**53 that lose data when converted
> to double, while still allowing for inputs larger than 2**63 via double)

JSON leaves defining limits on range and precision of numbers to
implementations.

Many implementations limit them to IEEE double precision.  Integers that
double can't represent exactly get rounded.  Whether you write an
integer as string of digits or the same sting followed by a redundant
exponent or fraction such as .0 doesn't matter.

We limit differently.  If the JSON number has neither a fraction nor an
exponent part, and it's representable as int64_t, then we parse it as
that.  Else we parse it as double.  See the big comment in
parse_literal().  Yes, the code there fails error handling 101.

Buys us exact representation of more integers at the price of subtly
different interpretation of JSON numbers with neither fraction nor
exponent part.

A few examples:

    JSON                    QTYPE_    value                  exact?
    1                       QINT      1                      yes
    1.0                     QFLOAT    1.0                    yes
    10000000000000001.0     QFLOAT    10000000000000000.0    no
    10000000000000001       QINT      10000000000000001      yes
    9223372036854775807     QINT      9223372036854775807    yes
    9223372036854775808     QFLOAT    9223372036854775808.0  yes
    9223372036854775809     QFLOAT    9223372036854775808.0  no
   -9223372036854775808     QINT     -9223372036854775808    yes

Yes, that means the integers in [2^63,2^64-1] are parsed as double.
uint64 in the schema is a pious lie.

>>>>  The format of a success response is:
>>>>
>>>> -{ "return": json-object, "id": json-value }
>>>> +{ "return": json-entity, "id": json-value }
>>>
>>> Unlike the other json-FOOs we use, "entity" isn't defined in RFC4627.
>>> "value" is, and we already use json-value.  What's the difference
>>> between the two?
>> 
>> Umm, when I wrote that, I was thinking "id":json-value meant integer, so
>> I wanted something that was not an integer.  But sure enough, json-value
>> is precisely the term I wanted to use:
>
> Well, given above at [1] that 'null' is a valid json-value but NOT
> accepted by our parser, I guess we are not quite accurate here.

I regard the parser choking on null as a bug.  Bugs are implementation
detail ;)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]