From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH v10 01/25] tests/qapi-schema: Test for reserved names, empty struct
Date: Thu, 22 Oct 2015 23:09:34 -0600 [thread overview]
Message-ID: <1445576998-2921-2-git-send-email-eblake@redhat.com> (raw)
In-Reply-To: <1445576998-2921-1-git-send-email-eblake@redhat.com>
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
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
--
2.4.3
next prev parent reply other threads:[~2015-10-23 5:10 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-23 5:09 [Qemu-devel] [PATCH v10 00/25] qapi collision reduction (post-introspection subset B') Eric Blake
2015-10-23 5:09 ` Eric Blake [this message]
2015-10-23 12:33 ` [Qemu-devel] [PATCH v10 01/25] tests/qapi-schema: Test for reserved names, empty struct Markus Armbruster
2015-10-23 12:39 ` Eric Blake
2015-10-23 14:17 ` Markus Armbruster
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 02/25] qapi: More idiomatic string operations Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 03/25] qapi: More robust conditions for when labels are needed Eric Blake
2015-10-23 12:44 ` Markus Armbruster
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 04/25] qapi: Reserve '*List' type names for list types Eric Blake
2015-10-23 12:53 ` Markus Armbruster
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 05/25] qapi: Reserve 'q_*' and 'has_*' member names Eric Blake
2015-10-23 13:05 ` Markus Armbruster
2015-10-23 14:14 ` Eric Blake
2015-10-23 14:47 ` Markus Armbruster
2015-10-23 14:52 ` Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 06/25] vnc: Hoist allocation of VncBasicInfo to callers Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 07/25] qapi-visit: Split off visit_type_FOO_fields forward decl Eric Blake
2015-10-23 13:46 ` Markus Armbruster
2015-10-23 14:35 ` Eric Blake
2015-10-23 18:05 ` Markus Armbruster
2015-10-23 19:44 ` Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 08/25] qapi-types: Refactor base fields output Eric Blake
2015-10-23 15:06 ` Markus Armbruster
2015-10-23 15:16 ` Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi base classes Eric Blake
2015-10-23 15:30 ` Markus Armbruster
2015-10-23 20:44 ` Eric Blake
2015-10-26 7:33 ` Markus Armbruster
2015-10-26 16:24 ` Eric Blake
2015-10-26 17:54 ` Markus Armbruster
2015-10-26 20:53 ` Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 10/25] qapi: Unbox base members Eric Blake
2015-10-23 19:14 ` Markus Armbruster
2015-10-23 19:19 ` Eric Blake
2015-10-23 20:45 ` Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 11/25] qapi-visit: Remove redundant functions for flat union base Eric Blake
2015-10-23 19:35 ` Markus Armbruster
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 12/25] qapi: Start converting to new qapi union layout Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 13/25] qapi-visit: Convert " Eric Blake
2015-10-26 17:07 ` Markus Armbruster
2015-10-26 20:39 ` Eric Blake
2015-10-27 7:08 ` Markus Armbruster
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 14/25] tests: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 15/25] block: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 16/25] sockets: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 17/25] net: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 18/25] char: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 19/25] input: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 20/25] memory: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 21/25] tpm: " Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 22/25] qapi: Finish converting " Eric Blake
2015-10-27 8:33 ` Markus Armbruster
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 23/25] qapi: Reserve 'u' member name Eric Blake
2015-10-26 17:27 ` Markus Armbruster
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 24/25] qapi: Remove outdated tests related to QMP/branch collisions Eric Blake
2015-10-23 23:27 ` Eric Blake
2015-10-23 5:09 ` [Qemu-devel] [PATCH v10 25/25] qapi: Simplify gen_struct_field() Eric Blake
2015-10-26 17:55 ` [Qemu-devel] [PATCH v10 00/25] qapi collision reduction (post-introspection subset B') Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1445576998-2921-2-git-send-email-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).