Re: [Qemu-devel] [PATCH 3/4] qobject: Parse non-finite numbers, as an extension

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 06/16/2016 10:25 AM, Markus Armbruster wrote:
>> I think this commit mixes up parsing of non-finite numbers, which we may
>> or may not want, with general test improvements, which we'll want
>> regardless.  Please split the patch.
>> 
>> On the parsing of non-finite numbers: the code looks good to me, but as
>> long as we're not ready to extend QMP to include non-finite numbers both
>> ways, I doubt we need to parse them.
>
> Even if we want to guarantee that we will never generate a non-finite
> number as the result of a QMP command, we DO have to worry about the
> 'id' field.  With all this series applied:
>
> $  ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp
> stdio{"QMP": {"version": {"qemu": {"micro": 50, "minor": 6, "major": 2},
> "package": " (v2.6.0-1151-g1bb9f5e)"}, "capabilities": []}}
> {'execute':'qmp_capabilities', 'id':[nan,inf,-Infinity]}
> {"return": {}, "id": ["nan", "inf", "-inf"]}

You get a different id back, in violation of the QMP spec.

> and with just 1-3 applied:
>
> $ ./x86_64-softmmu/qemu-system-x86_64 -nodefaults -nographic -qmp
> stdio{"QMP": {"version": {"qemu": {"micro": 50, "minor": 6, "major": 2},
> "package": " (v2.6.0-1151-g1bb9f5e)"}, "capabilities": []}}
> {'execute':'qmp_capabilities', 'id':[nan,inf,-Infinity]}
> {"return": {}, "id": [nan, inf, -inf]}

This would conform to a revision of the QMP spec that supports
non-finite numbers.  With the current spec, it's in "garbage in, garbage
out" territory.  At least it's the same garbage :)

Current master:

    {"QMP": {"version": {"qemu": {"micro": 50, "minor": 6, "major": 2}, "package": " (v2.6.0-1114-g585fcd4)"}, "capabilities": []}}
    {'execute':'qmp_capabilities', 'id':[nan,inf,-Infinity]}
    {"error": {"class": "GenericError", "desc": "Invalid JSON syntax"}}
    {"error": {"class": "GenericError", "desc": "Invalid JSON syntax"}}
    {"error": {"class": "GenericError", "desc": "Invalid JSON syntax"}}
    {"error": {"class": "GenericError", "desc": "Invalid JSON syntax"}}

Our diagnosis and reporting of lexical errors sucks, but that's not
news.

> [okay, I cheated, as evidenced by the new git stuff appended in the
> version output being the same between the two lines, but you get the idea].
>
> I'm comfortable with a compromise that asserts that QMP commands will
> never output a non-finite number anywhere that introspection lists
> 'number', but that QMP itself understands the extension of parsing a
> bare inf anywhere, and if a bare inf appears under 'id', then the replay
> of 'id' will also include a bare inf (if your JSON library allows you to
> send non-JSON, it should also allow you to parse non-JSON).

Yes.

However, non-finite numbers have never worked in input.  I don't see why
we should make them work in input now, unless we extend QMP to include
non-finite numbers, and make them work both ways.