| Message ID | 1411165504-18198-13-git-send-email-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > Demonstrate that the qapi generator silently parses confusing > types, which may cause other errors later on. Later patches > will update the expected results as the generator is made stricter. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > tests/Makefile | 8 ++++++-- > tests/qapi-schema/data-array-empty.err | 0 > tests/qapi-schema/data-array-empty.exit | 1 + > tests/qapi-schema/data-array-empty.json | 2 ++ [Twelve new tests...] > create mode 100644 tests/qapi-schema/returns-unknown.err > create mode 100644 tests/qapi-schema/returns-unknown.exit > create mode 100644 tests/qapi-schema/returns-unknown.json > create mode 100644 tests/qapi-schema/returns-unknown.out Holy moly! > diff --git a/tests/Makefile b/tests/Makefile > index 5e01952..6fe34f7 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -203,8 +203,12 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ > double-data.json unknown-expr-key.json redefined-type.json \ > redefined-command.json redefined-builtin.json redefined-event.json \ > type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \ > - missing-colon.json missing-comma-list.json \ > - missing-comma-object.json non-objects.json \ > + data-array-empty.json data-array-unknown.json data-int.json \ > + data-unknown.json data-member-unknown.json data-member-array.json \ > + data-member-array-bad.json returns-array-bad.json returns-int.json \ > + returns-unknown.json missing-colon.json missing-comma-list.json \ > + missing-comma-object.json nested-struct-data.json \ > + nested-struct-returns.json non-objects.json \ > qapi-schema-test.json quoted-structural-chars.json \ > trailing-comma-list.json trailing-comma-object.json \ > unclosed-list.json unclosed-object.json unclosed-string.json \ > diff --git a/tests/qapi-schema/data-array-empty.err b/tests/qapi-schema/data-array-empty.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/data-array-empty.exit b/tests/qapi-schema/data-array-empty.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/data-array-empty.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/data-array-empty.json b/tests/qapi-schema/data-array-empty.json > new file mode 100644 > index 0000000..41b6c1e > --- /dev/null > +++ b/tests/qapi-schema/data-array-empty.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject an array for data if it does not contain a known type > +{ 'command': 'oops', 'data': [ ] } Do we want to permit anything but a complex type for 'data'? For what it's worth, docs/qmp/qmp-spec.txt specifies: 2.3 Issuing Commands -------------------- The format for command execution is: { "execute": json-string, "arguments": json-object, "id": json-value } Where, - The "execute" member identifies the command to be executed by the Server - The "arguments" member is used to pass any arguments required for the execution of the command, it is optional when no arguments are required - The "id" member is a transaction identification associated with the command execution, it is optional and will be part of the response if provided The QAPI schema's 'data' is "arguments" on the wire. Semantically, 'data' of a complex type / json-object "arguments" is an ordered list of named parameters. Makes obvious sense. 'data' of list type / json-array "arguments" is an ordered list of unnamed parameters. Makes sense, but it isn't how QMP works. Or C for that matter. Do we really want to support this in QAPI? If yes, then 'data': [] means the same thing as 'data': {} or no 'data': no arguments. Aside: discussion of list types in qapi-code-gen.txt is spread over the places that use them. I feel giving them their own section on the same level as complex types etc. would make sense. 'data' of built-in or enumeration type / json-number or json-string "arguments" could be regarded as a single unnamed parameter. Even if we want unnamed parameters, the question remains whether we want two syntactic forms for a single unnamed parameter, boxed in a [ ] and unboxed. I doubt it. One kind of types left to discuss: unions. I figure the semantics of a simple or flat union type are obvious enough, so we can discuss whether we want them. Anonymous unions are a different matter, because they boil down to a single parameter that need not be json-object. > diff --git a/tests/qapi-schema/data-array-empty.out b/tests/qapi-schema/data-array-empty.out > new file mode 100644 > index 0000000..67802be > --- /dev/null > +++ b/tests/qapi-schema/data-array-empty.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('data', [])])] > +[] > +[] > diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/data-array-unknown.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/data-array-unknown.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/data-array-unknown.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/data-array-unknown.json b/tests/qapi-schema/data-array-unknown.json > new file mode 100644 > index 0000000..434fb5f > --- /dev/null > +++ b/tests/qapi-schema/data-array-unknown.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject an array for data if it does not contain a known type > +{ 'command': 'oops', 'data': [ 'NoSuchType' ] } > diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/data-array-unknown.out > new file mode 100644 > index 0000000..c3bc05e > --- /dev/null > +++ b/tests/qapi-schema/data-array-unknown.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])] > +[] > +[] > diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/data-int.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/data-int.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/data-int.json b/tests/qapi-schema/data-int.json > new file mode 100644 > index 0000000..37916e0 > --- /dev/null > +++ b/tests/qapi-schema/data-int.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject commands where data is not an array or complex type > +{ 'command': 'oops', 'data': 'int' } > diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/data-int.out > new file mode 100644 > index 0000000..e589a4f > --- /dev/null > +++ b/tests/qapi-schema/data-int.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('data', 'int')])] > +[] > +[] > diff --git a/tests/qapi-schema/data-member-array-bad.err b/tests/qapi-schema/data-member-array-bad.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/data-member-array-bad.exit b/tests/qapi-schema/data-member-array-bad.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/data-member-array-bad.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/data-member-array-bad.json b/tests/qapi-schema/data-member-array-bad.json > new file mode 100644 > index 0000000..c954af1 > --- /dev/null > +++ b/tests/qapi-schema/data-member-array-bad.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject data if it does not contain a valid array type > +{ 'command': 'oops', 'data': { 'member': [ { 'nested': 'str' } ] } } I'm probably just suffering from temporary denseness here... why is this example problematic? To me, it looks like a single argument 'member' of type "array of complex type with a single member 'nested' of builtin-type 'str'". > diff --git a/tests/qapi-schema/data-member-array-bad.out b/tests/qapi-schema/data-member-array-bad.out > new file mode 100644 > index 0000000..0e00c41 > --- /dev/null > +++ b/tests/qapi-schema/data-member-array-bad.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', [OrderedDict([('nested', 'str')])])]))])] > +[] > +[] > diff --git a/tests/qapi-schema/data-member-array.err b/tests/qapi-schema/data-member-array.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/data-member-array.exit b/tests/qapi-schema/data-member-array.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/data-member-array.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/data-member-array.json b/tests/qapi-schema/data-member-array.json > new file mode 100644 > index 0000000..7cce276 > --- /dev/null > +++ b/tests/qapi-schema/data-member-array.json > @@ -0,0 +1,4 @@ > +# valid array members > +{ 'enum': 'abc', 'data': [ 'a', 'b', 'c' ] } > +{ 'type': 'def', 'data': { 'array': [ 'abc' ] } } > +{ 'command': 'okay', 'data': { 'member1': [ 'int' ], 'member2': [ 'def' ] } } > diff --git a/tests/qapi-schema/data-member-array.out b/tests/qapi-schema/data-member-array.out > new file mode 100644 > index 0000000..8287120 > --- /dev/null > +++ b/tests/qapi-schema/data-member-array.out > @@ -0,0 +1,5 @@ > +[OrderedDict([('enum', 'abc'), ('data', ['a', 'b', 'c'])]), > + OrderedDict([('type', 'def'), ('data', OrderedDict([('array', ['abc'])]))]), > + OrderedDict([('command', 'okay'), ('data', OrderedDict([('member1', ['int']), ('member2', ['def'])]))])] > +[{'enum_name': 'abc', 'enum_values': ['a', 'b', 'c']}] > +[OrderedDict([('type', 'def'), ('data', OrderedDict([('array', ['abc'])]))])] > diff --git a/tests/qapi-schema/data-member-unknown.err b/tests/qapi-schema/data-member-unknown.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/data-member-unknown.exit b/tests/qapi-schema/data-member-unknown.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/data-member-unknown.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/data-member-unknown.json b/tests/qapi-schema/data-member-unknown.json > new file mode 100644 > index 0000000..40e6252 > --- /dev/null > +++ b/tests/qapi-schema/data-member-unknown.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject data if it does not contain a known type > +{ 'command': 'oops', 'data': { 'member': 'NoSuchType' } } > diff --git a/tests/qapi-schema/data-member-unknown.out b/tests/qapi-schema/data-member-unknown.out > new file mode 100644 > index 0000000..36252a5 > --- /dev/null > +++ b/tests/qapi-schema/data-member-unknown.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', 'NoSuchType')]))])] > +[] > +[] > diff --git a/tests/qapi-schema/data-unknown.err b/tests/qapi-schema/data-unknown.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/data-unknown.exit b/tests/qapi-schema/data-unknown.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/data-unknown.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/data-unknown.json b/tests/qapi-schema/data-unknown.json > new file mode 100644 > index 0000000..776bd34 > --- /dev/null > +++ b/tests/qapi-schema/data-unknown.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject data if it does not contain a known type > +{ 'command': 'oops', 'data': 'NoSuchType' } > diff --git a/tests/qapi-schema/data-unknown.out b/tests/qapi-schema/data-unknown.out > new file mode 100644 > index 0000000..2c60726 > --- /dev/null > +++ b/tests/qapi-schema/data-unknown.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])] > +[] > +[] > diff --git a/tests/qapi-schema/nested-struct-data.err b/tests/qapi-schema/nested-struct-data.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/nested-struct-data.exit b/tests/qapi-schema/nested-struct-data.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/nested-struct-data.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json > new file mode 100644 > index 0000000..0247c8c > --- /dev/null > +++ b/tests/qapi-schema/nested-struct-data.json > @@ -0,0 +1,4 @@ > +# FIXME: inline subtypes collide with our desired future use of defaults > +{ 'command': 'foo', > + 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' }, > + 'returns': {} } > diff --git a/tests/qapi-schema/nested-struct-data.out b/tests/qapi-schema/nested-struct-data.out > new file mode 100644 > index 0000000..999cbb8 > --- /dev/null > +++ b/tests/qapi-schema/nested-struct-data.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'foo'), ('data', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')])), ('returns', OrderedDict())])] > +[] > +[] > diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/nested-struct-returns.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/nested-struct-returns.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json > new file mode 100644 > index 0000000..5a46840 > --- /dev/null > +++ b/tests/qapi-schema/nested-struct-returns.json > @@ -0,0 +1,3 @@ > +# FIXME: inline subtypes collide with our desired future use of defaults > +{ 'command': 'foo', > + 'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } > diff --git a/tests/qapi-schema/nested-struct-returns.out b/tests/qapi-schema/nested-struct-returns.out > new file mode 100644 > index 0000000..c53d23b > --- /dev/null > +++ b/tests/qapi-schema/nested-struct-returns.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'foo'), ('returns', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')]))])] > +[] > +[] > diff --git a/tests/qapi-schema/returns-array-bad.err b/tests/qapi-schema/returns-array-bad.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/returns-array-bad.exit b/tests/qapi-schema/returns-array-bad.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/returns-array-bad.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/returns-array-bad.json b/tests/qapi-schema/returns-array-bad.json > new file mode 100644 > index 0000000..14882c1 > --- /dev/null > +++ b/tests/qapi-schema/returns-array-bad.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject an array return that is not a single type > +{ 'command': 'oops', 'returns': [ 'str', 'str' ] } Do we want to permit anything but a complex type for 'returns'? For what it's worth, docs/qmp/qmp-spec.txt specifies: 2.4.1 success ------------- The format of a success response is: { "return": json-object, "id": json-value } Where, - The "return" member contains the command returned data, which is defined in a per-command basis or an empty json-object if the command does not return data - The "id" member contains the transaction identification associated with the command execution if issued by the Client The QAPI schema's 'returns' becomes "return" on the wire. We suck. qmp-spec.txt is *wrong*! We actually use json-array in addition to json-object. Similar argument on types wanted as for 'data' / "arguments" above. I think we should permit exactly the same QAPI types, plus lists. > diff --git a/tests/qapi-schema/returns-array-bad.out b/tests/qapi-schema/returns-array-bad.out > new file mode 100644 > index 0000000..eccad55 > --- /dev/null > +++ b/tests/qapi-schema/returns-array-bad.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('returns', ['str', 'str'])])] > +[] > +[] > diff --git a/tests/qapi-schema/returns-int.err b/tests/qapi-schema/returns-int.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/returns-int.exit b/tests/qapi-schema/returns-int.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/returns-int.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/returns-int.json b/tests/qapi-schema/returns-int.json > new file mode 100644 > index 0000000..7888fb1 > --- /dev/null > +++ b/tests/qapi-schema/returns-int.json > @@ -0,0 +1,2 @@ > +# It is okay (although not extensible) to return a non-dictionary > +{ 'command': 'okay', 'returns': 'int' } > diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out > new file mode 100644 > index 0000000..36b00a9 > --- /dev/null > +++ b/tests/qapi-schema/returns-int.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'okay'), ('returns', 'int')])] > +[] > +[] > diff --git a/tests/qapi-schema/returns-unknown.err b/tests/qapi-schema/returns-unknown.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/returns-unknown.exit b/tests/qapi-schema/returns-unknown.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/returns-unknown.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/returns-unknown.json b/tests/qapi-schema/returns-unknown.json > new file mode 100644 > index 0000000..61f20eb > --- /dev/null > +++ b/tests/qapi-schema/returns-unknown.json > @@ -0,0 +1,2 @@ > +# FIXME: we should reject returns if it does not contain a known type > +{ 'command': 'oops', 'returns': 'NoSuchType' } > diff --git a/tests/qapi-schema/returns-unknown.out b/tests/qapi-schema/returns-unknown.out > new file mode 100644 > index 0000000..a482c83 > --- /dev/null > +++ b/tests/qapi-schema/returns-unknown.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])] > +[] > +[] scripts/qapi* is a sick joke when it comes to semantic analysis.
Markus Armbruster <armbru@redhat.com> writes: > Eric Blake <eblake@redhat.com> writes: > >> Demonstrate that the qapi generator silently parses confusing >> types, which may cause other errors later on. Later patches >> will update the expected results as the generator is made stricter. [...] >> diff --git a/tests/qapi-schema/returns-array-bad.json >> b/tests/qapi-schema/returns-array-bad.json >> new file mode 100644 >> index 0000000..14882c1 >> --- /dev/null >> +++ b/tests/qapi-schema/returns-array-bad.json >> @@ -0,0 +1,2 @@ >> +# FIXME: we should reject an array return that is not a single type >> +{ 'command': 'oops', 'returns': [ 'str', 'str' ] } Yes, we want this test, and your remaining tests of 'returns' are fine, too. > Do we want to permit anything but a complex type for 'returns'? > > For what it's worth, docs/qmp/qmp-spec.txt specifies: > > 2.4.1 success > ------------- > > The format of a success response is: > > { "return": json-object, "id": json-value } > > Where, > > - The "return" member contains the command returned data, which is defined > in a per-command basis or an empty json-object if the command does not > return data > - The "id" member contains the transaction identification associated > with the command execution if issued by the Client > > The QAPI schema's 'returns' becomes "return" on the wire. We suck. > > qmp-spec.txt is *wrong*! We actually use json-array in addition to > json-object. Actually, we use json-int and json-str as well: query-migrate-cache-size, ringbuf-read, human-monitor-command. > Similar argument on types wanted as for 'data' / "arguments" above. I > think we should permit exactly the same QAPI types, plus lists. The similarity to 'data' just isn't there. Separate analysis needed. Any QAPI types that don't make sense, other than list with length != 1? [...]
On 09/25/2014 01:34 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> Demonstrate that the qapi generator silently parses confusing >> types, which may cause other errors later on. Later patches >> will update the expected results as the generator is made stricter. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> --- >> tests/Makefile | 8 ++++++-- >> tests/qapi-schema/data-array-empty.err | 0 >> tests/qapi-schema/data-array-empty.exit | 1 + >> tests/qapi-schema/data-array-empty.json | 2 ++ > [Twelve new tests...] >> create mode 100644 tests/qapi-schema/returns-unknown.err >> create mode 100644 tests/qapi-schema/returns-unknown.exit >> create mode 100644 tests/qapi-schema/returns-unknown.json >> create mode 100644 tests/qapi-schema/returns-unknown.out > > Holy moly! Yeah, this series cleans up a lot of cruft, which means a lot of testing. >> +++ b/tests/qapi-schema/data-array-empty.json >> @@ -0,0 +1,2 @@ >> +# FIXME: we should reject an array for data if it does not contain a known type >> +{ 'command': 'oops', 'data': [ ] } > > Do we want to permit anything but a complex type for 'data'? Oh, good question. Probably not (I just tested, and nothing already does that). I'll tighten it in v5 (mostly doc changes, plus a one-liner in 13/19 to remove allow_array=True when calling check_type for a command data member). > > For what it's worth, docs/qmp/qmp-spec.txt specifies: Ooh, I probably ought to skim that file when making my doc improvements on the front end of the series. > > 'data' of list type / json-array "arguments" is an ordered list of > unnamed parameters. Makes sense, but it isn't how QMP works. Or C for > that matter. Do we really want to support this in QAPI? No existing command takes a non-dict for "arguments", and the generator probably chokes on it. > > If yes, then 'data': [] means the same thing as 'data': {} or no 'data': > no arguments. > > Aside: discussion of list types in qapi-code-gen.txt is spread over the > places that use them. I feel giving them their own section on the same > level as complex types etc. would make sense. Good idea, will do in v5. > > 'data' of built-in or enumeration type / json-number or json-string > "arguments" could be regarded as a single unnamed parameter. Even if we > want unnamed parameters, the question remains whether we want two > syntactic forms for a single unnamed parameter, boxed in a [ ] and > unboxed. I doubt it. No. We don't (patch 13/19 already forbids them, with no violators found). It's not extensible (well, maybe it is by having some way to mark a dict so that at most one of its keys is the default key to be implied when used in a non-dict form, and all other keys being optional, but that's ugly). > > One kind of types left to discuss: unions. I figure the semantics of a > simple or flat union type are obvious enough, so we can discuss whether > we want them. Anonymous unions are a different matter, because they > boil down to a single parameter that need not be json-object. Oooh, I didn't even consider anon unions. We absolutely need to support { 'command': 'foo', 'data': 'FlatUnion' } (blockdev-add, anyone), but you are probably right that we don't want to support { 'command': 'foo', 'data': 'AnonUnion'}, because it would allow "arguments" to be a non-dictionary (unless the AnonUnion has only a dict branch, but then why make it a union?). I'll have to make qapi.py be smarter about regular vs. anon unions - it might be easier by using an actual different keyword for anon unions, because they are so different in nature. (Generated code will be the same, just a tweak to the qapi representation and to qapi.py). I'll play with that for v5. >> +++ b/tests/qapi-schema/data-member-array-bad.json >> @@ -0,0 +1,2 @@ >> +# FIXME: we should reject data if it does not contain a valid array type >> +{ 'command': 'oops', 'data': { 'member': [ { 'nested': 'str' } ] } } > > I'm probably just suffering from temporary denseness here... why is this > example problematic? To me, it looks like a single argument 'member' of > type "array of complex type with a single member 'nested' of > builtin-type 'str'". The generator does not have a way to produce a List of an unnamed type. All lists are of named types (or rather, every creation of a named type generates code for both that type and its list counterpart). Maybe we eventually want to support an array of an anonymous type, but the generator doesn't handle it now. So it was easier to forbid it when writing 13/19. >> +# FIXME: we should reject an array return that is not a single type >> +{ 'command': 'oops', 'returns': [ 'str', 'str' ] } > > Do we want to permit anything but a complex type for 'returns'? Sadly, yes. We have existing commands that do just that. I already documented that new commands should avoid it (it's not extensible). > > The QAPI schema's 'returns' becomes "return" on the wire. We suck. We could search-and-replace the schema, but why bother. Yeah, the discrepancy is a bit annoying; on the other hand, it makes it easy to tell schema apart from on-the-wire samples, at least for commands that return something :) > > qmp-spec.txt is *wrong*! We actually use json-array in addition to > json-object. Yep, added to my list of doc improvements for v5. >> +++ b/tests/qapi-schema/returns-unknown.out >> @@ -0,0 +1,3 @@ >> +[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])] >> +[] >> +[] > > scripts/qapi* is a sick joke when it comes to semantic analysis. That gets a lot better in 13/19 :)
On 09/25/2014 02:06 AM, Markus Armbruster wrote: >> >> The QAPI schema's 'returns' becomes "return" on the wire. We suck. >> >> qmp-spec.txt is *wrong*! We actually use json-array in addition to >> json-object. > > Actually, we use json-int and json-str as well: > query-migrate-cache-size, ringbuf-read, human-monitor-command. > >> Similar argument on types wanted as for 'data' / "arguments" above. I >> think we should permit exactly the same QAPI types, plus lists. > > The similarity to 'data' just isn't there. Separate analysis needed. Correct. 'data' and 'returns' are different beasts when it comes to acceptable types. And different still from the acceptable type of each member of a dictionary. But my check_type function in 13/19 is flexible enough to cover all the cases. > > Any QAPI types that don't make sense, other than list with length != 1? Return of an anon union isn't used yet, but _might_ make sense (as the only feasible way of changing existing commands that return an array or primitive extensible to instead return a dict) - except that back-compat demands that we can't return a dict in place of a primitive unless the arguments of the command are also enhanced (that is, older callers are not expecting a dict, so we can't return a dict unless the caller witnesses they are new enough by explicitly asking for a dict return).
Eric Blake <eblake@redhat.com> writes: > On 09/25/2014 01:34 AM, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >>> Demonstrate that the qapi generator silently parses confusing >>> types, which may cause other errors later on. Later patches >>> will update the expected results as the generator is made stricter. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >>> --- >>> tests/Makefile | 8 ++++++-- >>> tests/qapi-schema/data-array-empty.err | 0 >>> tests/qapi-schema/data-array-empty.exit | 1 + >>> tests/qapi-schema/data-array-empty.json | 2 ++ >> [Twelve new tests...] >>> create mode 100644 tests/qapi-schema/returns-unknown.err >>> create mode 100644 tests/qapi-schema/returns-unknown.exit >>> create mode 100644 tests/qapi-schema/returns-unknown.json >>> create mode 100644 tests/qapi-schema/returns-unknown.out >> >> Holy moly! > > Yeah, this series cleans up a lot of cruft, which means a lot of testing. Very much appreciated. >>> +++ b/tests/qapi-schema/data-array-empty.json >>> @@ -0,0 +1,2 @@ >>> +# FIXME: we should reject an array for data if it does not contain >>> a known type >>> +{ 'command': 'oops', 'data': [ ] } >> >> Do we want to permit anything but a complex type for 'data'? > > Oh, good question. Probably not (I just tested, and nothing already > does that). I'll tighten it in v5 (mostly doc changes, plus a one-liner > in 13/19 to remove allow_array=True when calling check_type for a > command data member). Yes, please. >> For what it's worth, docs/qmp/qmp-spec.txt specifies: > > Ooh, I probably ought to skim that file when making my doc improvements > on the front end of the series. > >> 'data' of list type / json-array "arguments" is an ordered list of >> unnamed parameters. Makes sense, but it isn't how QMP works. Or C for >> that matter. Do we really want to support this in QAPI? > > No existing command takes a non-dict for "arguments", and the generator > probably chokes on it. Let's stick to dict arguments. >> If yes, then 'data': [] means the same thing as 'data': {} or no 'data': >> no arguments. >> >> Aside: discussion of list types in qapi-code-gen.txt is spread over the >> places that use them. I feel giving them their own section on the same >> level as complex types etc. would make sense. > > Good idea, will do in v5. > >> >> 'data' of built-in or enumeration type / json-number or json-string >> "arguments" could be regarded as a single unnamed parameter. Even if we >> want unnamed parameters, the question remains whether we want two >> syntactic forms for a single unnamed parameter, boxed in a [ ] and >> unboxed. I doubt it. > > No. We don't (patch 13/19 already forbids them, with no violators > found). It's not extensible (well, maybe it is by having some way to > mark a dict so that at most one of its keys is the default key to be > implied when used in a non-dict form, and all other keys being optional, > but that's ugly). Agreed. >> One kind of types left to discuss: unions. I figure the semantics of a >> simple or flat union type are obvious enough, so we can discuss whether >> we want them. Anonymous unions are a different matter, because they >> boil down to a single parameter that need not be json-object. > > Oooh, I didn't even consider anon unions. We absolutely need to support > { 'command': 'foo', 'data': 'FlatUnion' } (blockdev-add, anyone), but > you are probably right that we don't want to support { 'command': 'foo', > 'data': 'AnonUnion'}, because it would allow "arguments" to be a > non-dictionary (unless the AnonUnion has only a dict branch, but then > why make it a union?). I'll have to make qapi.py be smarter about > regular vs. anon unions - it might be easier by using an actual > different keyword for anon unions, because they are so different in > nature. (Generated code will be the same, just a tweak to the qapi > representation and to qapi.py). I'll play with that for v5. Okay :) >>> +++ b/tests/qapi-schema/data-member-array-bad.json >>> @@ -0,0 +1,2 @@ >>> +# FIXME: we should reject data if it does not contain a valid array type >>> +{ 'command': 'oops', 'data': { 'member': [ { 'nested': 'str' } ] } } >> >> I'm probably just suffering from temporary denseness here... why is this >> example problematic? To me, it looks like a single argument 'member' of >> type "array of complex type with a single member 'nested' of >> builtin-type 'str'". > > The generator does not have a way to produce a List of an unnamed type. > All lists are of named types (or rather, every creation of a named type > generates code for both that type and its list counterpart). Maybe we > eventually want to support an array of an anonymous type, but the > generator doesn't handle it now. So it was easier to forbid it when > writing 13/19. I see. We already accepted restricting nested structs (see series subject), and this is just one instance. >>> +# FIXME: we should reject an array return that is not a single type >>> +{ 'command': 'oops', 'returns': [ 'str', 'str' ] } >> >> Do we want to permit anything but a complex type for 'returns'? > > Sadly, yes. We have existing commands that do just that. I already > documented that new commands should avoid it (it's not extensible). If we care, we can whitelist the existing offenders, and reject new offenders. >> The QAPI schema's 'returns' becomes "return" on the wire. We suck. > > We could search-and-replace the schema, but why bother. Yeah, the > discrepancy is a bit annoying; on the other hand, it makes it easy to > tell schema apart from on-the-wire samples, at least for commands that > return something :) > >> >> qmp-spec.txt is *wrong*! We actually use json-array in addition to >> json-object. > > Yep, added to my list of doc improvements for v5. > > >>> +++ b/tests/qapi-schema/returns-unknown.out >>> @@ -0,0 +1,3 @@ >>> +[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])] >>> +[] >>> +[] >> >> scripts/qapi* is a sick joke when it comes to semantic analysis. > > That gets a lot better in 13/19 :) Will review as soon as I can!
Eric Blake <eblake@redhat.com> writes: > On 09/25/2014 02:06 AM, Markus Armbruster wrote: > >>> >>> The QAPI schema's 'returns' becomes "return" on the wire. We suck. >>> >>> qmp-spec.txt is *wrong*! We actually use json-array in addition to >>> json-object. >> >> Actually, we use json-int and json-str as well: >> query-migrate-cache-size, ringbuf-read, human-monitor-command. >> >>> Similar argument on types wanted as for 'data' / "arguments" above. I >>> think we should permit exactly the same QAPI types, plus lists. >> >> The similarity to 'data' just isn't there. Separate analysis needed. > > Correct. 'data' and 'returns' are different beasts when it comes to > acceptable types. And different still from the acceptable type of each > member of a dictionary. But my check_type function in 13/19 is flexible > enough to cover all the cases. > >> >> Any QAPI types that don't make sense, other than list with length != 1? > > Return of an anon union isn't used yet, but _might_ make sense (as the > only feasible way of changing existing commands that return an array or > primitive extensible to instead return a dict) - Good point. > except that back-compat > demands that we can't return a dict in place of a primitive unless the > arguments of the command are also enhanced (that is, older callers are > not expecting a dict, so we can't return a dict unless the caller > witnesses they are new enough by explicitly asking for a dict return). I think we can keep things simple for now and reject anonymous unions. We can always relax the check when we run into a use. You're giving the generator a good shove from "god knows what it accepts, but as long as you stick to stuff that is being used already, probably generates something that works" towards "if it accepts it, it works". I like it.
On 09/25/2014 10:12 AM, Markus Armbruster wrote: >>> Do we want to permit anything but a complex type for 'returns'? >> >> Sadly, yes. We have existing commands that do just that. I already >> documented that new commands should avoid it (it's not extensible). > > If we care, we can whitelist the existing offenders, and reject new > offenders. Also a good idea. The whitelist may grow over time, but forcing a developer to modify the whitelist calls attention to their action :) I'll add that to my v5 queue. Thanks again for a thought-provoking review.
[revisiting this series, finally!] On 09/25/2014 10:19 AM, Markus Armbruster wrote: >> Return of an anon union isn't used yet, but _might_ make sense (as the >> only feasible way of changing existing commands that return an array or >> primitive extensible to instead return a dict) - > > Good point. > >> except that back-compat >> demands that we can't return a dict in place of a primitive unless the >> arguments of the command are also enhanced (that is, older callers are >> not expecting a dict, so we can't return a dict unless the caller >> witnesses they are new enough by explicitly asking for a dict return). > > I think we can keep things simple for now and reject anonymous unions. > We can always relax the check when we run into a use. In trying to code up what it would take to easily reject anonymous unions from a return type, I'm realizing that it would be smarter to quit mixing anonymous unions in with other unions. Refresher course: on the wire, both a regular union: QAPI: { 'type': 'Type1', 'data': { 'value': 'int' } } { 'union': 'Foo', 'data': { 'a': 'Type1', 'b': 'Type2' } } { 'command': 'bar', 'data': 'Foo' } Wire: { "execute": "bar", "arguments": { "type": "a", "data": { "value": 1 } } } and a flat union: QAPI: { 'type': 'Type1', 'data': { 'value': 'int' } } { 'enum': 'Enum', 'data': [ 'a', 'b' ] } { 'type': 'Base', { 'switch': 'Enum' } } { 'union': 'Foo', 'base': 'Base', 'discriminator': 'switch', 'data': { 'a': 'Type1', 'b': 'Type2' } } { 'command': 'bar', 'data': 'Foo' } Wire: { "execute": "bar", "arguments": { "switch": "a", "value": 1 } } happen to guarantee a top-level dictionary (plain unions always have a two-element dictionary, flat unions are required to have a base type which is itself a dictionary). But an anonymous union is explicitly allowing a multi-type variable, where the determination of which branch of the union is made by the type of the variable. Furthermore, we do not allow two branches to have the same type, so at least one branch must be a non-dictionary; but as _all_ QMP commands currently take a dictionary for the "arguments" key, we do not want to allow: QAPI: { 'type': 'Type1', 'data': { 'value': 'int' } } { 'union': 'Foo', 'discriminator': {}, 'data': { 'a': 'Type1', 'b': 'int' } } { 'command': 'bar', 'data': 'Foo' } Wire: { "execute": "bar", "arguments": 1 } Tracking all three qapi expressions as union types is making the generator code a bit verbose, especially since the code generation for all three is so distinct. Proposal: I am proposing that we convert to an alternate syntax for what we now term as anonymous unions. It will not have any impact to the wire representation of QMP, only to the qapi code generators. The proposal is simple: instead of using "'union':'Name', 'discriminator':{}", we instead use "'alternate': 'Foo'" when declaring a type as an anonymous union (which, for obvious reasons, I would then update the documentation to call an "alternate" instead of an "anonymous union"). I'll go ahead and propose the patches (I've already done the bulk of the conversion work, to prove that not many files were affected).
Cc'ing Kevin as fair punishment for is previous work on QAPI code generation in general, and union types in particular. Eric Blake <eblake@redhat.com> writes: > [revisiting this series, finally!] > > On 09/25/2014 10:19 AM, Markus Armbruster wrote: > >>> Return of an anon union isn't used yet, but _might_ make sense (as the >>> only feasible way of changing existing commands that return an array or >>> primitive extensible to instead return a dict) - >> >> Good point. >> >>> except that back-compat >>> demands that we can't return a dict in place of a primitive unless the >>> arguments of the command are also enhanced (that is, older callers are >>> not expecting a dict, so we can't return a dict unless the caller >>> witnesses they are new enough by explicitly asking for a dict return). >> >> I think we can keep things simple for now and reject anonymous unions. >> We can always relax the check when we run into a use. > > In trying to code up what it would take to easily reject anonymous > unions from a return type, I'm realizing that it would be smarter to > quit mixing anonymous unions in with other unions. > > Refresher course: on the wire, both a regular union: > > QAPI: > { 'type': 'Type1', 'data': { 'value': 'int' } } > { 'union': 'Foo', 'data': { 'a': 'Type1', 'b': 'Type2' } } > { 'command': 'bar', 'data': 'Foo' } > Wire: > { "execute": "bar", "arguments": { "type": "a", > "data": { "value": 1 } } } > > and a flat union: > > QAPI: > { 'type': 'Type1', 'data': { 'value': 'int' } } > { 'enum': 'Enum', 'data': [ 'a', 'b' ] } > { 'type': 'Base', { 'switch': 'Enum' } } > { 'union': 'Foo', 'base': 'Base', 'discriminator': 'switch', > 'data': { 'a': 'Type1', 'b': 'Type2' } } > { 'command': 'bar', 'data': 'Foo' } > Wire: > { "execute": "bar", "arguments": { "switch": "a", > "value": 1 } } > > happen to guarantee a top-level dictionary (plain unions always have a > two-element dictionary, flat unions are required to have a base type > which is itself a dictionary). Yes. > But an anonymous union is explicitly > allowing a multi-type variable, where the determination of which branch > of the union is made by the type of the variable. Furthermore, we do > not allow two branches to have the same type, Even more severe: the JSON types have to be different! Thus, at most one can be a complex or union type, and at most one can be a string or enum type. > so at least one branch > must be a non-dictionary; Correct. > but as _all_ QMP commands currently take a > dictionary for the "arguments" key, we do not want to allow: > > QAPI: > { 'type': 'Type1', 'data': { 'value': 'int' } } > { 'union': 'Foo', 'discriminator': {}, > 'data': { 'a': 'Type1', 'b': 'int' } } > { 'command': 'bar', 'data': 'Foo' } > Wire: > { "execute": "bar", "arguments": 1 } Yes, let's stay out of the generalization tar pits and stick to named parameters, i.e. JSON object arguments. > Tracking all three qapi expressions as union types is making the > generator code a bit verbose, especially since the code generation for > all three is so distinct. > > > Proposal: I am proposing that we convert to an alternate syntax for what > we now term as anonymous unions. It will not have any impact to the > wire representation of QMP, only to the qapi code generators. The > proposal is simple: instead of using "'union':'Name', > 'discriminator':{}", we instead use "'alternate': 'Foo'" when declaring > a type as an anonymous union (which, for obvious reasons, I would then > update the documentation to call an "alternate" instead of an "anonymous > union"). This separates tagged and untagged unions more clearly: reserve 'union' for the two kinds of tagged unions, switch the untagged union to 'alternate'. I don't mind. Kevin, any objections? > I'll go ahead and propose the patches (I've already done the bulk of the > conversion work, to prove that not many files were affected). I'll gladly review.
diff --git a/tests/Makefile b/tests/Makefile index 5e01952..6fe34f7 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -203,8 +203,12 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ double-data.json unknown-expr-key.json redefined-type.json \ redefined-command.json redefined-builtin.json redefined-event.json \ type-bypass.json type-bypass-no-gen.json type-bypass-bad-gen.json \ - missing-colon.json missing-comma-list.json \ - missing-comma-object.json non-objects.json \ + data-array-empty.json data-array-unknown.json data-int.json \ + data-unknown.json data-member-unknown.json data-member-array.json \ + data-member-array-bad.json returns-array-bad.json returns-int.json \ + returns-unknown.json missing-colon.json missing-comma-list.json \ + missing-comma-object.json nested-struct-data.json \ + nested-struct-returns.json non-objects.json \ qapi-schema-test.json quoted-structural-chars.json \ trailing-comma-list.json trailing-comma-object.json \ unclosed-list.json unclosed-object.json unclosed-string.json \ diff --git a/tests/qapi-schema/data-array-empty.err b/tests/qapi-schema/data-array-empty.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/data-array-empty.exit b/tests/qapi-schema/data-array-empty.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/data-array-empty.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/data-array-empty.json b/tests/qapi-schema/data-array-empty.json new file mode 100644 index 0000000..41b6c1e --- /dev/null +++ b/tests/qapi-schema/data-array-empty.json @@ -0,0 +1,2 @@ +# FIXME: we should reject an array for data if it does not contain a known type +{ 'command': 'oops', 'data': [ ] } diff --git a/tests/qapi-schema/data-array-empty.out b/tests/qapi-schema/data-array-empty.out new file mode 100644 index 0000000..67802be --- /dev/null +++ b/tests/qapi-schema/data-array-empty.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('data', [])])] +[] +[] diff --git a/tests/qapi-schema/data-array-unknown.err b/tests/qapi-schema/data-array-unknown.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/data-array-unknown.exit b/tests/qapi-schema/data-array-unknown.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/data-array-unknown.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/data-array-unknown.json b/tests/qapi-schema/data-array-unknown.json new file mode 100644 index 0000000..434fb5f --- /dev/null +++ b/tests/qapi-schema/data-array-unknown.json @@ -0,0 +1,2 @@ +# FIXME: we should reject an array for data if it does not contain a known type +{ 'command': 'oops', 'data': [ 'NoSuchType' ] } diff --git a/tests/qapi-schema/data-array-unknown.out b/tests/qapi-schema/data-array-unknown.out new file mode 100644 index 0000000..c3bc05e --- /dev/null +++ b/tests/qapi-schema/data-array-unknown.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('data', ['NoSuchType'])])] +[] +[] diff --git a/tests/qapi-schema/data-int.err b/tests/qapi-schema/data-int.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/data-int.exit b/tests/qapi-schema/data-int.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/data-int.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/data-int.json b/tests/qapi-schema/data-int.json new file mode 100644 index 0000000..37916e0 --- /dev/null +++ b/tests/qapi-schema/data-int.json @@ -0,0 +1,2 @@ +# FIXME: we should reject commands where data is not an array or complex type +{ 'command': 'oops', 'data': 'int' } diff --git a/tests/qapi-schema/data-int.out b/tests/qapi-schema/data-int.out new file mode 100644 index 0000000..e589a4f --- /dev/null +++ b/tests/qapi-schema/data-int.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('data', 'int')])] +[] +[] diff --git a/tests/qapi-schema/data-member-array-bad.err b/tests/qapi-schema/data-member-array-bad.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/data-member-array-bad.exit b/tests/qapi-schema/data-member-array-bad.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/data-member-array-bad.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/data-member-array-bad.json b/tests/qapi-schema/data-member-array-bad.json new file mode 100644 index 0000000..c954af1 --- /dev/null +++ b/tests/qapi-schema/data-member-array-bad.json @@ -0,0 +1,2 @@ +# FIXME: we should reject data if it does not contain a valid array type +{ 'command': 'oops', 'data': { 'member': [ { 'nested': 'str' } ] } } diff --git a/tests/qapi-schema/data-member-array-bad.out b/tests/qapi-schema/data-member-array-bad.out new file mode 100644 index 0000000..0e00c41 --- /dev/null +++ b/tests/qapi-schema/data-member-array-bad.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', [OrderedDict([('nested', 'str')])])]))])] +[] +[] diff --git a/tests/qapi-schema/data-member-array.err b/tests/qapi-schema/data-member-array.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/data-member-array.exit b/tests/qapi-schema/data-member-array.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/data-member-array.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/data-member-array.json b/tests/qapi-schema/data-member-array.json new file mode 100644 index 0000000..7cce276 --- /dev/null +++ b/tests/qapi-schema/data-member-array.json @@ -0,0 +1,4 @@ +# valid array members +{ 'enum': 'abc', 'data': [ 'a', 'b', 'c' ] } +{ 'type': 'def', 'data': { 'array': [ 'abc' ] } } +{ 'command': 'okay', 'data': { 'member1': [ 'int' ], 'member2': [ 'def' ] } } diff --git a/tests/qapi-schema/data-member-array.out b/tests/qapi-schema/data-member-array.out new file mode 100644 index 0000000..8287120 --- /dev/null +++ b/tests/qapi-schema/data-member-array.out @@ -0,0 +1,5 @@ +[OrderedDict([('enum', 'abc'), ('data', ['a', 'b', 'c'])]), + OrderedDict([('type', 'def'), ('data', OrderedDict([('array', ['abc'])]))]), + OrderedDict([('command', 'okay'), ('data', OrderedDict([('member1', ['int']), ('member2', ['def'])]))])] +[{'enum_name': 'abc', 'enum_values': ['a', 'b', 'c']}] +[OrderedDict([('type', 'def'), ('data', OrderedDict([('array', ['abc'])]))])] diff --git a/tests/qapi-schema/data-member-unknown.err b/tests/qapi-schema/data-member-unknown.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/data-member-unknown.exit b/tests/qapi-schema/data-member-unknown.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/data-member-unknown.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/data-member-unknown.json b/tests/qapi-schema/data-member-unknown.json new file mode 100644 index 0000000..40e6252 --- /dev/null +++ b/tests/qapi-schema/data-member-unknown.json @@ -0,0 +1,2 @@ +# FIXME: we should reject data if it does not contain a known type +{ 'command': 'oops', 'data': { 'member': 'NoSuchType' } } diff --git a/tests/qapi-schema/data-member-unknown.out b/tests/qapi-schema/data-member-unknown.out new file mode 100644 index 0000000..36252a5 --- /dev/null +++ b/tests/qapi-schema/data-member-unknown.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('data', OrderedDict([('member', 'NoSuchType')]))])] +[] +[] diff --git a/tests/qapi-schema/data-unknown.err b/tests/qapi-schema/data-unknown.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/data-unknown.exit b/tests/qapi-schema/data-unknown.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/data-unknown.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/data-unknown.json b/tests/qapi-schema/data-unknown.json new file mode 100644 index 0000000..776bd34 --- /dev/null +++ b/tests/qapi-schema/data-unknown.json @@ -0,0 +1,2 @@ +# FIXME: we should reject data if it does not contain a known type +{ 'command': 'oops', 'data': 'NoSuchType' } diff --git a/tests/qapi-schema/data-unknown.out b/tests/qapi-schema/data-unknown.out new file mode 100644 index 0000000..2c60726 --- /dev/null +++ b/tests/qapi-schema/data-unknown.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('data', 'NoSuchType')])] +[] +[] diff --git a/tests/qapi-schema/nested-struct-data.err b/tests/qapi-schema/nested-struct-data.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/nested-struct-data.exit b/tests/qapi-schema/nested-struct-data.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/nested-struct-data.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/nested-struct-data.json b/tests/qapi-schema/nested-struct-data.json new file mode 100644 index 0000000..0247c8c --- /dev/null +++ b/tests/qapi-schema/nested-struct-data.json @@ -0,0 +1,4 @@ +# FIXME: inline subtypes collide with our desired future use of defaults +{ 'command': 'foo', + 'data': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' }, + 'returns': {} } diff --git a/tests/qapi-schema/nested-struct-data.out b/tests/qapi-schema/nested-struct-data.out new file mode 100644 index 0000000..999cbb8 --- /dev/null +++ b/tests/qapi-schema/nested-struct-data.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'foo'), ('data', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')])), ('returns', OrderedDict())])] +[] +[] diff --git a/tests/qapi-schema/nested-struct-returns.err b/tests/qapi-schema/nested-struct-returns.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/nested-struct-returns.exit b/tests/qapi-schema/nested-struct-returns.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/nested-struct-returns.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/nested-struct-returns.json b/tests/qapi-schema/nested-struct-returns.json new file mode 100644 index 0000000..5a46840 --- /dev/null +++ b/tests/qapi-schema/nested-struct-returns.json @@ -0,0 +1,3 @@ +# FIXME: inline subtypes collide with our desired future use of defaults +{ 'command': 'foo', + 'returns': { 'a' : { 'string' : 'str', 'integer': 'int' }, 'b' : 'str' } } diff --git a/tests/qapi-schema/nested-struct-returns.out b/tests/qapi-schema/nested-struct-returns.out new file mode 100644 index 0000000..c53d23b --- /dev/null +++ b/tests/qapi-schema/nested-struct-returns.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'foo'), ('returns', OrderedDict([('a', OrderedDict([('string', 'str'), ('integer', 'int')])), ('b', 'str')]))])] +[] +[] diff --git a/tests/qapi-schema/returns-array-bad.err b/tests/qapi-schema/returns-array-bad.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/returns-array-bad.exit b/tests/qapi-schema/returns-array-bad.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/returns-array-bad.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/returns-array-bad.json b/tests/qapi-schema/returns-array-bad.json new file mode 100644 index 0000000..14882c1 --- /dev/null +++ b/tests/qapi-schema/returns-array-bad.json @@ -0,0 +1,2 @@ +# FIXME: we should reject an array return that is not a single type +{ 'command': 'oops', 'returns': [ 'str', 'str' ] } diff --git a/tests/qapi-schema/returns-array-bad.out b/tests/qapi-schema/returns-array-bad.out new file mode 100644 index 0000000..eccad55 --- /dev/null +++ b/tests/qapi-schema/returns-array-bad.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('returns', ['str', 'str'])])] +[] +[] diff --git a/tests/qapi-schema/returns-int.err b/tests/qapi-schema/returns-int.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/returns-int.exit b/tests/qapi-schema/returns-int.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/returns-int.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/returns-int.json b/tests/qapi-schema/returns-int.json new file mode 100644 index 0000000..7888fb1 --- /dev/null +++ b/tests/qapi-schema/returns-int.json @@ -0,0 +1,2 @@ +# It is okay (although not extensible) to return a non-dictionary +{ 'command': 'okay', 'returns': 'int' } diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out new file mode 100644 index 0000000..36b00a9 --- /dev/null +++ b/tests/qapi-schema/returns-int.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'okay'), ('returns', 'int')])] +[] +[] diff --git a/tests/qapi-schema/returns-unknown.err b/tests/qapi-schema/returns-unknown.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/returns-unknown.exit b/tests/qapi-schema/returns-unknown.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/returns-unknown.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/returns-unknown.json b/tests/qapi-schema/returns-unknown.json new file mode 100644 index 0000000..61f20eb --- /dev/null +++ b/tests/qapi-schema/returns-unknown.json @@ -0,0 +1,2 @@ +# FIXME: we should reject returns if it does not contain a known type +{ 'command': 'oops', 'returns': 'NoSuchType' } diff --git a/tests/qapi-schema/returns-unknown.out b/tests/qapi-schema/returns-unknown.out new file mode 100644 index 0000000..a482c83 --- /dev/null +++ b/tests/qapi-schema/returns-unknown.out @@ -0,0 +1,3 @@ +[OrderedDict([('command', 'oops'), ('returns', 'NoSuchType')])] +[] +[]
Demonstrate that the qapi generator silently parses confusing types, which may cause other errors later on. Later patches will update the expected results as the generator is made stricter. Signed-off-by: Eric Blake <eblake@redhat.com> --- tests/Makefile | 8 ++++++-- tests/qapi-schema/data-array-empty.err | 0 tests/qapi-schema/data-array-empty.exit | 1 + tests/qapi-schema/data-array-empty.json | 2 ++ tests/qapi-schema/data-array-empty.out | 3 +++ tests/qapi-schema/data-array-unknown.err | 0 tests/qapi-schema/data-array-unknown.exit | 1 + tests/qapi-schema/data-array-unknown.json | 2 ++ tests/qapi-schema/data-array-unknown.out | 3 +++ tests/qapi-schema/data-int.err | 0 tests/qapi-schema/data-int.exit | 1 + tests/qapi-schema/data-int.json | 2 ++ tests/qapi-schema/data-int.out | 3 +++ tests/qapi-schema/data-member-array-bad.err | 0 tests/qapi-schema/data-member-array-bad.exit | 1 + tests/qapi-schema/data-member-array-bad.json | 2 ++ tests/qapi-schema/data-member-array-bad.out | 3 +++ tests/qapi-schema/data-member-array.err | 0 tests/qapi-schema/data-member-array.exit | 1 + tests/qapi-schema/data-member-array.json | 4 ++++ tests/qapi-schema/data-member-array.out | 5 +++++ tests/qapi-schema/data-member-unknown.err | 0 tests/qapi-schema/data-member-unknown.exit | 1 + tests/qapi-schema/data-member-unknown.json | 2 ++ tests/qapi-schema/data-member-unknown.out | 3 +++ tests/qapi-schema/data-unknown.err | 0 tests/qapi-schema/data-unknown.exit | 1 + tests/qapi-schema/data-unknown.json | 2 ++ tests/qapi-schema/data-unknown.out | 3 +++ tests/qapi-schema/nested-struct-data.err | 0 tests/qapi-schema/nested-struct-data.exit | 1 + tests/qapi-schema/nested-struct-data.json | 4 ++++ tests/qapi-schema/nested-struct-data.out | 3 +++ tests/qapi-schema/nested-struct-returns.err | 0 tests/qapi-schema/nested-struct-returns.exit | 1 + tests/qapi-schema/nested-struct-returns.json | 3 +++ tests/qapi-schema/nested-struct-returns.out | 3 +++ tests/qapi-schema/returns-array-bad.err | 0 tests/qapi-schema/returns-array-bad.exit | 1 + tests/qapi-schema/returns-array-bad.json | 2 ++ tests/qapi-schema/returns-array-bad.out | 3 +++ tests/qapi-schema/returns-int.err | 0 tests/qapi-schema/returns-int.exit | 1 + tests/qapi-schema/returns-int.json | 2 ++ tests/qapi-schema/returns-int.out | 3 +++ tests/qapi-schema/returns-unknown.err | 0 tests/qapi-schema/returns-unknown.exit | 1 + tests/qapi-schema/returns-unknown.json | 2 ++ tests/qapi-schema/returns-unknown.out | 3 +++ 49 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 tests/qapi-schema/data-array-empty.err create mode 100644 tests/qapi-schema/data-array-empty.exit create mode 100644 tests/qapi-schema/data-array-empty.json create mode 100644 tests/qapi-schema/data-array-empty.out create mode 100644 tests/qapi-schema/data-array-unknown.err create mode 100644 tests/qapi-schema/data-array-unknown.exit create mode 100644 tests/qapi-schema/data-array-unknown.json create mode 100644 tests/qapi-schema/data-array-unknown.out create mode 100644 tests/qapi-schema/data-int.err create mode 100644 tests/qapi-schema/data-int.exit create mode 100644 tests/qapi-schema/data-int.json create mode 100644 tests/qapi-schema/data-int.out create mode 100644 tests/qapi-schema/data-member-array-bad.err create mode 100644 tests/qapi-schema/data-member-array-bad.exit create mode 100644 tests/qapi-schema/data-member-array-bad.json create mode 100644 tests/qapi-schema/data-member-array-bad.out create mode 100644 tests/qapi-schema/data-member-array.err create mode 100644 tests/qapi-schema/data-member-array.exit create mode 100644 tests/qapi-schema/data-member-array.json create mode 100644 tests/qapi-schema/data-member-array.out create mode 100644 tests/qapi-schema/data-member-unknown.err create mode 100644 tests/qapi-schema/data-member-unknown.exit create mode 100644 tests/qapi-schema/data-member-unknown.json create mode 100644 tests/qapi-schema/data-member-unknown.out create mode 100644 tests/qapi-schema/data-unknown.err create mode 100644 tests/qapi-schema/data-unknown.exit create mode 100644 tests/qapi-schema/data-unknown.json create mode 100644 tests/qapi-schema/data-unknown.out create mode 100644 tests/qapi-schema/nested-struct-data.err create mode 100644 tests/qapi-schema/nested-struct-data.exit create mode 100644 tests/qapi-schema/nested-struct-data.json create mode 100644 tests/qapi-schema/nested-struct-data.out create mode 100644 tests/qapi-schema/nested-struct-returns.err create mode 100644 tests/qapi-schema/nested-struct-returns.exit create mode 100644 tests/qapi-schema/nested-struct-returns.json create mode 100644 tests/qapi-schema/nested-struct-returns.out create mode 100644 tests/qapi-schema/returns-array-bad.err create mode 100644 tests/qapi-schema/returns-array-bad.exit create mode 100644 tests/qapi-schema/returns-array-bad.json create mode 100644 tests/qapi-schema/returns-array-bad.out create mode 100644 tests/qapi-schema/returns-int.err create mode 100644 tests/qapi-schema/returns-int.exit create mode 100644 tests/qapi-schema/returns-int.json create mode 100644 tests/qapi-schema/returns-int.out create mode 100644 tests/qapi-schema/returns-unknown.err create mode 100644 tests/qapi-schema/returns-unknown.exit create mode 100644 tests/qapi-schema/returns-unknown.json create mode 100644 tests/qapi-schema/returns-unknown.out