[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v14 15/15] qapi: Detect base class loops
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v14 15/15] qapi: Detect base class loops |
Date: |
Tue, 1 Dec 2015 22:20:59 -0700 |
It should be fairly obvious that qapi base classes need to
form an acyclic graph, since QMP cannot specify the same
key more than once, while base classes are included as flat
members alongside other members added by the child. But the
old check_member_clash() parser function was not prepared to
check for this, and entered an infinite recursion (at least
until Python gives up, complaining about nesting too deep).
Now that check_member_clash() has been recently removed,
attempts at self-inheritance trigger an assertion failure
introduced by commit ac88219a. The obvious fix is to turn
the assertion into a conditional.
This patch includes both the tests (base-cycle-direct and
base-cycle-indirect) and the fix, since the .err file output
for the unfixed case is not useful (particularly when it was
warning about unbounded recursion, as that limit may be
platform-specific).
We don't need to worry about cycles in flat unions (neither
the base type nor the type of a variant can be a union) nor
in alternates (alternate branches cannot themselves be an
alternate). But if we later allow a union type as a variant,
we will still be okay, as QAPISchemaObjectTypeVariants.check()
triggers the same QAPISchemaObjectType.check() that will
detect any loops.
Likewise, we need not worry about the case of diamond
inheritance where the same class is used for a flat union base
class and one of its variants; either both uses will introduce
a collision in trying to insert the same member name twice, or
the shared type is empty and changes nothing.
Signed-off-by: Eric Blake <address@hidden>
---
v14: no change
v13 (no v12): simplify error message, use cuter name 'Loopy' in
testsuite
v11 (no v10): rename base-cycle to base-cycle-indirect, and add
base-cycle-direct; touch up commit message
v9: no change
v8: improve commit message
v7: improve commit message
v6: rebase to earlier info changes
---
scripts/qapi.py | 4 +++-
tests/Makefile | 2 ++
tests/qapi-schema/base-cycle-direct.err | 1 +
tests/qapi-schema/base-cycle-direct.exit | 1 +
tests/qapi-schema/base-cycle-direct.json | 2 ++
tests/qapi-schema/base-cycle-direct.out | 0
tests/qapi-schema/base-cycle-indirect.err | 1 +
tests/qapi-schema/base-cycle-indirect.exit | 1 +
tests/qapi-schema/base-cycle-indirect.json | 3 +++
tests/qapi-schema/base-cycle-indirect.out | 0
10 files changed, 14 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/base-cycle-direct.err
create mode 100644 tests/qapi-schema/base-cycle-direct.exit
create mode 100644 tests/qapi-schema/base-cycle-direct.json
create mode 100644 tests/qapi-schema/base-cycle-direct.out
create mode 100644 tests/qapi-schema/base-cycle-indirect.err
create mode 100644 tests/qapi-schema/base-cycle-indirect.exit
create mode 100644 tests/qapi-schema/base-cycle-indirect.json
create mode 100644 tests/qapi-schema/base-cycle-indirect.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0a616fc..56c8fb1 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -941,7 +941,9 @@ class QAPISchemaObjectType(QAPISchemaType):
self.members = None
def check(self, schema):
- assert self.members is not False # not running in cycles
+ if self.members is False: # check for cycles
+ raise QAPIExprError(self.info,
+ "Object %s contains itself" % self.name)
if self.members:
return
self.members = False # mark as being checked
diff --git a/tests/Makefile b/tests/Makefile
index 25bd6b4..477cc85 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -257,6 +257,8 @@ qapi-schema += bad-ident.json
qapi-schema += bad-type-bool.json
qapi-schema += bad-type-dict.json
qapi-schema += bad-type-int.json
+qapi-schema += base-cycle-direct.json
+qapi-schema += base-cycle-indirect.json
qapi-schema += command-int.json
qapi-schema += comments.json
qapi-schema += double-data.json
diff --git a/tests/qapi-schema/base-cycle-direct.err
b/tests/qapi-schema/base-cycle-direct.err
new file mode 100644
index 0000000..9c68f65
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-direct.err
@@ -0,0 +1 @@
+tests/qapi-schema/base-cycle-direct.json:2: Object Loopy contains itself
diff --git a/tests/qapi-schema/base-cycle-direct.exit
b/tests/qapi-schema/base-cycle-direct.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-direct.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/base-cycle-direct.json
b/tests/qapi-schema/base-cycle-direct.json
new file mode 100644
index 0000000..4fc66d0
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-direct.json
@@ -0,0 +1,2 @@
+# we reject a loop in base classes
+{ 'struct': 'Loopy', 'base': 'Loopy', 'data': {} }
diff --git a/tests/qapi-schema/base-cycle-direct.out
b/tests/qapi-schema/base-cycle-direct.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/base-cycle-indirect.err
b/tests/qapi-schema/base-cycle-indirect.err
new file mode 100644
index 0000000..fc92fe4
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-indirect.err
@@ -0,0 +1 @@
+tests/qapi-schema/base-cycle-indirect.json:2: Object Base1 contains itself
diff --git a/tests/qapi-schema/base-cycle-indirect.exit
b/tests/qapi-schema/base-cycle-indirect.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-indirect.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/base-cycle-indirect.json
b/tests/qapi-schema/base-cycle-indirect.json
new file mode 100644
index 0000000..2866772
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-indirect.json
@@ -0,0 +1,3 @@
+# we reject a loop in base classes
+{ 'struct': 'Base1', 'base': 'Base2', 'data': {} }
+{ 'struct': 'Base2', 'base': 'Base1', 'data': {} }
diff --git a/tests/qapi-schema/base-cycle-indirect.out
b/tests/qapi-schema/base-cycle-indirect.out
new file mode 100644
index 0000000..e69de29
--
2.4.3
- [Qemu-devel] [PATCH v14 01/15] qobject: Simplify QObject, (continued)
- [Qemu-devel] [PATCH v14 01/15] qobject: Simplify QObject, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 07/15] qapi: Fix alternates that accept 'number' but not 'int', Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 09/15] qapi: Shorter visits of optional fields, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 02/15] qobject: Rename qtype_code to QType, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 10/15] qapi: Prepare new QAPISchemaMember base class, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 03/15] qapi: Convert QType into QAPI built-in enum type, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 08/15] qapi: Simplify visits of optional fields, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 11/15] qapi: Track enum values by QAPISchemaMember, not string, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH v14 15/15] qapi: Detect base class loops,
Eric Blake <=
- [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Eric Blake, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Markus Armbruster, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Eric Blake, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Eric Blake, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Markus Armbruster, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Eric Blake, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Markus Armbruster, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Markus Armbruster, 2015/12/02
- Re: [Qemu-devel] [PATCH v14 13/15] qapi: Enforce (or whitelist) case conventions on qapi members, Eric Blake, 2015/12/02
- [Qemu-devel] [PATCH] fixup! qapi: Enforce (or whitelist) case conventions on qapi members, Eric Blake, 2015/12/02