Eric Blake <ebl...@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.
