| Message ID | 1445576998-2921-2-git-send-email-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > We are failing to detect a collision between a QMP member and > the implicit 'has_*' flag for another optional QMP member. The > easiest fix would be for a future patch to reserve the entire > "has[-_]" namespace for member names (the collision is also > possible for branch names within flat unions, but only as long > as branch names can collide with QMP names; since future > patches are about to remove that, it is not worth testing here). This is args-has-clash.json. > A similar collision exists between a QMP member where c_name() > munges what might otherwise be a reserved name to start with > 'q_', and another member explicitly starts with "q[-_]". Again, > the easiest task for a future patch will be reserving the entire s/task/solution/ ? > namespace, but here for commands as well as members. This is reserved-member.json. > Our current representation of qapi arrays is done by appending > 'List' to the element name; but we are not preventing the > creation of a non-array type with the same name. This is struct-name-list.json. > Finally, our testsuite does not have any compilation coverage > of struct inheritance with empty qapi structs. This is qapi-schema-test.json. Left undescribed: reserved-commands.json :) > Add tests to cover these issues. > > On the other hand, there is currently no technical reason to > forbid type name patterns from member names, or member name > patterns from types, since the two are not in the same namespace > in C and won't collide (but it's not worth adding positive tests > of these corner cases, especially while there is other churn > pending in patches that rearrange which collisions actually > happen). > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v10: retitle, split off 'u' collisions and positive tests for > later, add 'q_' collisions and empty struct inheritance, improve > commit message, rename args-name-has to args-has-clash > v9: new patch > --- > tests/Makefile | 4 ++++ > tests/qapi-schema/args-has-clash.err | 0 > tests/qapi-schema/args-has-clash.exit | 1 + > tests/qapi-schema/args-has-clash.json | 6 ++++++ > tests/qapi-schema/args-has-clash.out | 6 ++++++ > tests/qapi-schema/qapi-schema-test.json | 4 ++++ > tests/qapi-schema/qapi-schema-test.out | 3 +++ > tests/qapi-schema/reserved-command.err | 0 > tests/qapi-schema/reserved-command.exit | 1 + > tests/qapi-schema/reserved-command.json | 7 +++++++ > tests/qapi-schema/reserved-command.out | 5 +++++ > tests/qapi-schema/reserved-member.err | 0 > tests/qapi-schema/reserved-member.exit | 1 + > tests/qapi-schema/reserved-member.json | 6 ++++++ > tests/qapi-schema/reserved-member.out | 4 ++++ > tests/qapi-schema/struct-name-list.err | 0 > tests/qapi-schema/struct-name-list.exit | 1 + > tests/qapi-schema/struct-name-list.json | 5 +++++ > tests/qapi-schema/struct-name-list.out | 3 +++ > 19 files changed, 57 insertions(+) > create mode 100644 tests/qapi-schema/args-has-clash.err > create mode 100644 tests/qapi-schema/args-has-clash.exit > create mode 100644 tests/qapi-schema/args-has-clash.json > create mode 100644 tests/qapi-schema/args-has-clash.out > create mode 100644 tests/qapi-schema/reserved-command.err > create mode 100644 tests/qapi-schema/reserved-command.exit > create mode 100644 tests/qapi-schema/reserved-command.json > create mode 100644 tests/qapi-schema/reserved-command.out > create mode 100644 tests/qapi-schema/reserved-member.err > create mode 100644 tests/qapi-schema/reserved-member.exit > create mode 100644 tests/qapi-schema/reserved-member.json > create mode 100644 tests/qapi-schema/reserved-member.out > create mode 100644 tests/qapi-schema/struct-name-list.err > create mode 100644 tests/qapi-schema/struct-name-list.exit > create mode 100644 tests/qapi-schema/struct-name-list.json > create mode 100644 tests/qapi-schema/struct-name-list.out > > diff --git a/tests/Makefile b/tests/Makefile > index 0531b30..cc6b24f 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -237,6 +237,7 @@ qapi-schema += args-alternate.json > qapi-schema += args-any.json > qapi-schema += args-array-empty.json > qapi-schema += args-array-unknown.json > +qapi-schema += args-has-clash.json > qapi-schema += args-int.json > qapi-schema += args-invalid.json > qapi-schema += args-member-array-bad.json > @@ -314,6 +315,8 @@ qapi-schema += redefined-builtin.json > qapi-schema += redefined-command.json > qapi-schema += redefined-event.json > qapi-schema += redefined-type.json > +qapi-schema += reserved-command.json > +qapi-schema += reserved-member.json > qapi-schema += returns-alternate.json > qapi-schema += returns-array-bad.json > qapi-schema += returns-dict.json > @@ -324,6 +327,7 @@ qapi-schema += struct-base-clash-deep.json > qapi-schema += struct-base-clash.json > qapi-schema += struct-data-invalid.json > qapi-schema += struct-member-invalid.json > +qapi-schema += struct-name-list.json > qapi-schema += trailing-comma-list.json > qapi-schema += trailing-comma-object.json > qapi-schema += type-bypass-bad-gen.json > diff --git a/tests/qapi-schema/args-has-clash.err b/tests/qapi-schema/args-has-clash.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/args-has-clash.exit b/tests/qapi-schema/args-has-clash.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/args-has-clash.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/args-has-clash.json b/tests/qapi-schema/args-has-clash.json > new file mode 100644 > index 0000000..a2197de > --- /dev/null > +++ b/tests/qapi-schema/args-has-clash.json > @@ -0,0 +1,6 @@ > +# C member name collision > +# FIXME - This parses, but fails to compile, because the C struct is given > +# two 'has_a' members, one from the flag for optional 'a', and the other > +# from member 'has-a'. Either reject this at parse time, or munge the C > +# names to avoid the collision. > +{ 'command': 'oops', 'data': { '*a': 'str', 'has-a': 'str' } } > diff --git a/tests/qapi-schema/args-has-clash.out b/tests/qapi-schema/args-has-clash.out > new file mode 100644 > index 0000000..5a18b6b > --- /dev/null > +++ b/tests/qapi-schema/args-has-clash.out > @@ -0,0 +1,6 @@ > +object :empty > +object :obj-oops-arg > + member a: str optional=True > + member has-a: str optional=False > +command oops :obj-oops-arg -> None > + gen=True success_response=True > diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json > index 4e2d7c2..48e104b 100644 > --- a/tests/qapi-schema/qapi-schema-test.json > +++ b/tests/qapi-schema/qapi-schema-test.json > @@ -11,6 +11,10 @@ > # An empty enum, although unusual, is currently acceptable > { 'enum': 'MyEnum', 'data': [ ] } > > +# Likewise for an empty struct, including an empty base > +{ 'struct': 'Empty1', 'data': { } } > +{ 'struct': 'Empty2', 'base': 'Empty1', 'data': { } } > + > # for testing override of default naming heuristic > { 'enum': 'QEnumTwo', > 'prefix': 'QENUM_TWO', > diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out > index a6c80e0..a7e9aab 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -81,6 +81,9 @@ event EVENT_A None > event EVENT_B None > event EVENT_C :obj-EVENT_C-arg > event EVENT_D :obj-EVENT_D-arg > +object Empty1 > +object Empty2 > + base Empty1 > enum EnumOne ['value1', 'value2', 'value3'] > object EventStructOne > member struct1: UserDefOne optional=False > diff --git a/tests/qapi-schema/reserved-command.err b/tests/qapi-schema/reserved-command.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/reserved-command.exit b/tests/qapi-schema/reserved-command.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/reserved-command.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/reserved-command.json b/tests/qapi-schema/reserved-command.json > new file mode 100644 > index 0000000..be9944c > --- /dev/null > +++ b/tests/qapi-schema/reserved-command.json > @@ -0,0 +1,7 @@ > +# C entity name collision > +# FIXME - This parses, but fails to compile, because it attempts to declare > +# two 'qmp_q_unix' functions (one for 'q-unix', the other because c_name() > +# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should > +# reject attempts to explicitly use 'q_' names, to reserve it for qapi. > +{ 'command': 'unix' } > +{ 'command': 'q-unix' } Note that mangling 'unix' to 'q-unix' is pretty pointless for command names, because their C name occurs only in identifiers qmp_CNAME() and qmp_marshal_CNAME(). > diff --git a/tests/qapi-schema/reserved-command.out b/tests/qapi-schema/reserved-command.out > new file mode 100644 > index 0000000..b31b38f > --- /dev/null > +++ b/tests/qapi-schema/reserved-command.out > @@ -0,0 +1,5 @@ > +object :empty > +command q-unix None -> None > + gen=True success_response=True > +command unix None -> None > + gen=True success_response=True > diff --git a/tests/qapi-schema/reserved-member.err b/tests/qapi-schema/reserved-member.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/reserved-member.exit b/tests/qapi-schema/reserved-member.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/reserved-member.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/reserved-member.json b/tests/qapi-schema/reserved-member.json > new file mode 100644 > index 0000000..1602ed3 > --- /dev/null > +++ b/tests/qapi-schema/reserved-member.json > @@ -0,0 +1,6 @@ > +# C member name collision > +# FIXME - This parses, but fails to compile, because it attempts to declare > +# two 'q_unix' members (one for 'q-unix', the other because c_name() > +# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should > +# reject attempts to explicitly use 'q_' names, to reserve it for qapi. > +{ 'struct': 'Foo', 'data': { 'unix':'int', 'q-unix':'bool' } } Unlike command names, member names actually occur by themselves, so some name mangling is required. A less ham-fisted approach would mangle *complete* identifiers, i.e. c_name('qmp_' + name) instead of 'qmp_' + c_name(name). Please feel free to stick to ham-fisted for now. We need to make progress flushing the queue. > diff --git a/tests/qapi-schema/reserved-member.out b/tests/qapi-schema/reserved-member.out > new file mode 100644 > index 0000000..0d8685a > --- /dev/null > +++ b/tests/qapi-schema/reserved-member.out > @@ -0,0 +1,4 @@ > +object :empty > +object Foo > + member unix: int optional=False > + member q-unix: bool optional=False > diff --git a/tests/qapi-schema/struct-name-list.err b/tests/qapi-schema/struct-name-list.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/struct-name-list.exit b/tests/qapi-schema/struct-name-list.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/struct-name-list.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/struct-name-list.json b/tests/qapi-schema/struct-name-list.json > new file mode 100644 > index 0000000..8ad38e6 > --- /dev/null > +++ b/tests/qapi-schema/struct-name-list.json > @@ -0,0 +1,5 @@ > +# Potential C name collision > +# FIXME - This parses and compiles on its own, but prevents the user from > +# creating a type named 'Foo' and using ['Foo'] for an array. We should > +# reject the use of any non-array type names ending in 'List'. > +{ 'struct': 'FooList', 'data': { 's': 'str' } } "Non-array type name" makes no sense when talking about the QAPI schema, because arrays don't have names there. > diff --git a/tests/qapi-schema/struct-name-list.out b/tests/qapi-schema/struct-name-list.out > new file mode 100644 > index 0000000..0406bfe > --- /dev/null > +++ b/tests/qapi-schema/struct-name-list.out > @@ -0,0 +1,3 @@ > +object :empty > +object FooList > + member s: str optional=False
On 10/23/2015 06:33 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> We are failing to detect a collision between a QMP member and >> the implicit 'has_*' flag for another optional QMP member. The >> easiest fix would be for a future patch to reserve the entire >> "has[-_]" namespace for member names (the collision is also >> possible for branch names within flat unions, but only as long >> as branch names can collide with QMP names; since future >> patches are about to remove that, it is not worth testing here). > > This is args-has-clash.json. > >> A similar collision exists between a QMP member where c_name() >> munges what might otherwise be a reserved name to start with >> 'q_', and another member explicitly starts with "q[-_]". Again, >> the easiest task for a future patch will be reserving the entire > > s/task/solution/ ? > Sounds better. >> namespace, but here for commands as well as members. > > This is reserved-member.json. And reserve-commands.json > >> Our current representation of qapi arrays is done by appending >> 'List' to the element name; but we are not preventing the >> creation of a non-array type with the same name. > > This is struct-name-list.json. > >> Finally, our testsuite does not have any compilation coverage >> of struct inheritance with empty qapi structs. > > This is qapi-schema-test.json. > > Left undescribed: reserved-commands.json :) No, the 'q_' paragraph covered both. But yes, mentioning the actual test names in the commit body can't hurt. > >> Add tests to cover these issues. >> >> On the other hand, there is currently no technical reason to >> forbid type name patterns from member names, or member name >> patterns from types, since the two are not in the same namespace >> in C and won't collide (but it's not worth adding positive tests >> of these corner cases, especially while there is other churn >> pending in patches that rearrange which collisions actually >> happen). >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> --- >> +++ b/tests/qapi-schema/reserved-command.json >> @@ -0,0 +1,7 @@ >> +# C entity name collision >> +# FIXME - This parses, but fails to compile, because it attempts to declare >> +# two 'qmp_q_unix' functions (one for 'q-unix', the other because c_name() >> +# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should >> +# reject attempts to explicitly use 'q_' names, to reserve it for qapi. >> +{ 'command': 'unix' } >> +{ 'command': 'q-unix' } > > Note that mangling 'unix' to 'q-unix' is pretty pointless for command > names, because their C name occurs only in identifiers qmp_CNAME() and > qmp_marshal_CNAME(). > Probably true, but it is the current implementation, and we DO have a compile time collision until (unless?) we bother to fix it. >> +++ b/tests/qapi-schema/reserved-member.json >> @@ -0,0 +1,6 @@ >> +# C member name collision >> +# FIXME - This parses, but fails to compile, because it attempts to declare >> +# two 'q_unix' members (one for 'q-unix', the other because c_name() >> +# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should >> +# reject attempts to explicitly use 'q_' names, to reserve it for qapi. >> +{ 'struct': 'Foo', 'data': { 'unix':'int', 'q-unix':'bool' } } > > Unlike command names, member names actually occur by themselves, so some > name mangling is required. > > A less ham-fisted approach would mangle *complete* identifiers, i.e. > c_name('qmp_' + name) instead of 'qmp_' + c_name(name). > > Please feel free to stick to ham-fisted for now. We need to make > progress flushing the queue. Indeed - cleaning up command name mangling can come at a later date, and maybe at that point we can decide 'q_' reservations for command names doesn't add any power, and relax it at that point. >> +++ b/tests/qapi-schema/struct-name-list.json >> @@ -0,0 +1,5 @@ >> +# Potential C name collision >> +# FIXME - This parses and compiles on its own, but prevents the user from >> +# creating a type named 'Foo' and using ['Foo'] for an array. We should >> +# reject the use of any non-array type names ending in 'List'. >> +{ 'struct': 'FooList', 'data': { 's': 'str' } } > > "Non-array type name" makes no sense when talking about the QAPI schema, > because arrays don't have names there. Suggestions? Maybe s/non-array//? The wording changes in 2/25 when the test starts working.
Eric Blake <eblake@redhat.com> writes: > On 10/23/2015 06:33 AM, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: [...] >>> +++ b/tests/qapi-schema/struct-name-list.json >>> @@ -0,0 +1,5 @@ >>> +# Potential C name collision >>> +# FIXME - This parses and compiles on its own, but prevents the user from >>> +# creating a type named 'Foo' and using ['Foo'] for an array. We should >>> +# reject the use of any non-array type names ending in 'List'. >>> +{ 'struct': 'FooList', 'data': { 's': 'str' } } >> >> "Non-array type name" makes no sense when talking about the QAPI schema, >> because arrays don't have names there. > > Suggestions? Maybe s/non-array//? The wording changes in 2/25 when the > test starts working. Dropping non-array works for me.
diff --git a/tests/Makefile b/tests/Makefile index 0531b30..cc6b24f 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -237,6 +237,7 @@ qapi-schema += args-alternate.json qapi-schema += args-any.json qapi-schema += args-array-empty.json qapi-schema += args-array-unknown.json +qapi-schema += args-has-clash.json qapi-schema += args-int.json qapi-schema += args-invalid.json qapi-schema += args-member-array-bad.json @@ -314,6 +315,8 @@ qapi-schema += redefined-builtin.json qapi-schema += redefined-command.json qapi-schema += redefined-event.json qapi-schema += redefined-type.json +qapi-schema += reserved-command.json +qapi-schema += reserved-member.json qapi-schema += returns-alternate.json qapi-schema += returns-array-bad.json qapi-schema += returns-dict.json @@ -324,6 +327,7 @@ qapi-schema += struct-base-clash-deep.json qapi-schema += struct-base-clash.json qapi-schema += struct-data-invalid.json qapi-schema += struct-member-invalid.json +qapi-schema += struct-name-list.json qapi-schema += trailing-comma-list.json qapi-schema += trailing-comma-object.json qapi-schema += type-bypass-bad-gen.json diff --git a/tests/qapi-schema/args-has-clash.err b/tests/qapi-schema/args-has-clash.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/args-has-clash.exit b/tests/qapi-schema/args-has-clash.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/args-has-clash.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/args-has-clash.json b/tests/qapi-schema/args-has-clash.json new file mode 100644 index 0000000..a2197de --- /dev/null +++ b/tests/qapi-schema/args-has-clash.json @@ -0,0 +1,6 @@ +# C member name collision +# FIXME - This parses, but fails to compile, because the C struct is given +# two 'has_a' members, one from the flag for optional 'a', and the other +# from member 'has-a'. Either reject this at parse time, or munge the C +# names to avoid the collision. +{ 'command': 'oops', 'data': { '*a': 'str', 'has-a': 'str' } } diff --git a/tests/qapi-schema/args-has-clash.out b/tests/qapi-schema/args-has-clash.out new file mode 100644 index 0000000..5a18b6b --- /dev/null +++ b/tests/qapi-schema/args-has-clash.out @@ -0,0 +1,6 @@ +object :empty +object :obj-oops-arg + member a: str optional=True + member has-a: str optional=False +command oops :obj-oops-arg -> None + gen=True success_response=True diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 4e2d7c2..48e104b 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -11,6 +11,10 @@ # An empty enum, although unusual, is currently acceptable { 'enum': 'MyEnum', 'data': [ ] } +# Likewise for an empty struct, including an empty base +{ 'struct': 'Empty1', 'data': { } } +{ 'struct': 'Empty2', 'base': 'Empty1', 'data': { } } + # for testing override of default naming heuristic { 'enum': 'QEnumTwo', 'prefix': 'QENUM_TWO', diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index a6c80e0..a7e9aab 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -81,6 +81,9 @@ event EVENT_A None event EVENT_B None event EVENT_C :obj-EVENT_C-arg event EVENT_D :obj-EVENT_D-arg +object Empty1 +object Empty2 + base Empty1 enum EnumOne ['value1', 'value2', 'value3'] object EventStructOne member struct1: UserDefOne optional=False diff --git a/tests/qapi-schema/reserved-command.err b/tests/qapi-schema/reserved-command.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/reserved-command.exit b/tests/qapi-schema/reserved-command.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/reserved-command.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/reserved-command.json b/tests/qapi-schema/reserved-command.json new file mode 100644 index 0000000..be9944c --- /dev/null +++ b/tests/qapi-schema/reserved-command.json @@ -0,0 +1,7 @@ +# C entity name collision +# FIXME - This parses, but fails to compile, because it attempts to declare +# two 'qmp_q_unix' functions (one for 'q-unix', the other because c_name() +# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should +# reject attempts to explicitly use 'q_' names, to reserve it for qapi. +{ 'command': 'unix' } +{ 'command': 'q-unix' } diff --git a/tests/qapi-schema/reserved-command.out b/tests/qapi-schema/reserved-command.out new file mode 100644 index 0000000..b31b38f --- /dev/null +++ b/tests/qapi-schema/reserved-command.out @@ -0,0 +1,5 @@ +object :empty +command q-unix None -> None + gen=True success_response=True +command unix None -> None + gen=True success_response=True diff --git a/tests/qapi-schema/reserved-member.err b/tests/qapi-schema/reserved-member.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/reserved-member.exit b/tests/qapi-schema/reserved-member.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/reserved-member.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/reserved-member.json b/tests/qapi-schema/reserved-member.json new file mode 100644 index 0000000..1602ed3 --- /dev/null +++ b/tests/qapi-schema/reserved-member.json @@ -0,0 +1,6 @@ +# C member name collision +# FIXME - This parses, but fails to compile, because it attempts to declare +# two 'q_unix' members (one for 'q-unix', the other because c_name() +# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should +# reject attempts to explicitly use 'q_' names, to reserve it for qapi. +{ 'struct': 'Foo', 'data': { 'unix':'int', 'q-unix':'bool' } } diff --git a/tests/qapi-schema/reserved-member.out b/tests/qapi-schema/reserved-member.out new file mode 100644 index 0000000..0d8685a --- /dev/null +++ b/tests/qapi-schema/reserved-member.out @@ -0,0 +1,4 @@ +object :empty +object Foo + member unix: int optional=False + member q-unix: bool optional=False diff --git a/tests/qapi-schema/struct-name-list.err b/tests/qapi-schema/struct-name-list.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/struct-name-list.exit b/tests/qapi-schema/struct-name-list.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/struct-name-list.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/struct-name-list.json b/tests/qapi-schema/struct-name-list.json new file mode 100644 index 0000000..8ad38e6 --- /dev/null +++ b/tests/qapi-schema/struct-name-list.json @@ -0,0 +1,5 @@ +# Potential C name collision +# FIXME - This parses and compiles on its own, but prevents the user from +# creating a type named 'Foo' and using ['Foo'] for an array. We should +# reject the use of any non-array type names ending in 'List'. +{ 'struct': 'FooList', 'data': { 's': 'str' } } diff --git a/tests/qapi-schema/struct-name-list.out b/tests/qapi-schema/struct-name-list.out new file mode 100644 index 0000000..0406bfe --- /dev/null +++ b/tests/qapi-schema/struct-name-list.out @@ -0,0 +1,3 @@ +object :empty +object FooList + member s: str optional=False
We are failing to detect a collision between a QMP member and the implicit 'has_*' flag for another optional QMP member. The easiest fix would be for a future patch to reserve the entire "has[-_]" namespace for member names (the collision is also possible for branch names within flat unions, but only as long as branch names can collide with QMP names; since future patches are about to remove that, it is not worth testing here). A similar collision exists between a QMP member where c_name() munges what might otherwise be a reserved name to start with 'q_', and another member explicitly starts with "q[-_]". Again, the easiest task for a future patch will be reserving the entire namespace, but here for commands as well as members. Our current representation of qapi arrays is done by appending 'List' to the element name; but we are not preventing the creation of a non-array type with the same name. Finally, our testsuite does not have any compilation coverage of struct inheritance with empty qapi structs. Add tests to cover these issues. On the other hand, there is currently no technical reason to forbid type name patterns from member names, or member name patterns from types, since the two are not in the same namespace in C and won't collide (but it's not worth adding positive tests of these corner cases, especially while there is other churn pending in patches that rearrange which collisions actually happen). Signed-off-by: Eric Blake <eblake@redhat.com> --- v10: retitle, split off 'u' collisions and positive tests for later, add 'q_' collisions and empty struct inheritance, improve commit message, rename args-name-has to args-has-clash v9: new patch --- tests/Makefile | 4 ++++ tests/qapi-schema/args-has-clash.err | 0 tests/qapi-schema/args-has-clash.exit | 1 + tests/qapi-schema/args-has-clash.json | 6 ++++++ tests/qapi-schema/args-has-clash.out | 6 ++++++ tests/qapi-schema/qapi-schema-test.json | 4 ++++ tests/qapi-schema/qapi-schema-test.out | 3 +++ tests/qapi-schema/reserved-command.err | 0 tests/qapi-schema/reserved-command.exit | 1 + tests/qapi-schema/reserved-command.json | 7 +++++++ tests/qapi-schema/reserved-command.out | 5 +++++ tests/qapi-schema/reserved-member.err | 0 tests/qapi-schema/reserved-member.exit | 1 + tests/qapi-schema/reserved-member.json | 6 ++++++ tests/qapi-schema/reserved-member.out | 4 ++++ tests/qapi-schema/struct-name-list.err | 0 tests/qapi-schema/struct-name-list.exit | 1 + tests/qapi-schema/struct-name-list.json | 5 +++++ tests/qapi-schema/struct-name-list.out | 3 +++ 19 files changed, 57 insertions(+) create mode 100644 tests/qapi-schema/args-has-clash.err create mode 100644 tests/qapi-schema/args-has-clash.exit create mode 100644 tests/qapi-schema/args-has-clash.json create mode 100644 tests/qapi-schema/args-has-clash.out create mode 100644 tests/qapi-schema/reserved-command.err create mode 100644 tests/qapi-schema/reserved-command.exit create mode 100644 tests/qapi-schema/reserved-command.json create mode 100644 tests/qapi-schema/reserved-command.out create mode 100644 tests/qapi-schema/reserved-member.err create mode 100644 tests/qapi-schema/reserved-member.exit create mode 100644 tests/qapi-schema/reserved-member.json create mode 100644 tests/qapi-schema/reserved-member.out create mode 100644 tests/qapi-schema/struct-name-list.err create mode 100644 tests/qapi-schema/struct-name-list.exit create mode 100644 tests/qapi-schema/struct-name-list.json create mode 100644 tests/qapi-schema/struct-name-list.out