[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] qobject: Parse non-finite numbers, as an ex
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] qobject: Parse non-finite numbers, as an extension |
Date: |
Fri, 17 Jun 2016 10:04:35 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> 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.
- Re: [Qemu-devel] [PATCH 4/4] qobject: Output valid JSON for non-finite numbers, (continued)
[Qemu-devel] [PATCH 2/4] checkpatch: There is no qemu_strtod(), Eric Blake, 2016/06/09
[Qemu-devel] [PATCH 3/4] qobject: Parse non-finite numbers, as an extension, Eric Blake, 2016/06/09