[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v11 28/28] qapi: Detect base class loops
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v11 28/28] qapi: Detect base class loops |
Date: |
Tue, 10 Nov 2015 23:51:30 -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 test 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 nor a variant class can be a union) nor in alternates
(alternate branches cannot themselves be an alternate). And
even the case of using the same class for a flat union base
class and one its variants is already caught by the fact that
both uses will introduce the same member name (or will be okay
if the class is empty).
Signed-off-by: Eric Blake <address@hidden>
---
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 | 6 +++++-
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, 16 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 08a366e..e60c1d8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -941,7 +941,11 @@ 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
+ assert self._base_name
+ raise QAPIExprError(self.info,
+ "Object %s cyclically depends on %s"
+ % (self.name, self._base_name))
if self.members:
return
self.members = False # mark as being checked
diff --git a/tests/Makefile b/tests/Makefile
index cdff7a4..2706126 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -253,6 +253,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..46fc436
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-direct.err
@@ -0,0 +1 @@
+tests/qapi-schema/base-cycle-direct.json:2: Object Foo cyclically depends on
Foo
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..dff6fab
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-direct.json
@@ -0,0 +1,2 @@
+# we reject a loop in base classes
+{ 'struct': 'Foo', 'base': 'Foo', '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..9a1ca44
--- /dev/null
+++ b/tests/qapi-schema/base-cycle-indirect.err
@@ -0,0 +1 @@
+tests/qapi-schema/base-cycle-indirect.json:2: Object Base1 cyclically depends
on Base2
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 v11 26/28] qapi: Move duplicate member checks to schema check(), (continued)
- [Qemu-devel] [PATCH v11 22/28] qapi: Simplify visiting of alternate types, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 28/28] qapi: Detect base class loops,
Eric Blake <=
- [Qemu-devel] [PATCH v11 23/28] qapi: Fix alternates that accept 'number' but not 'int', Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 27/28] qapi: Move duplicate enum value checks to schema check(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Eric Blake, 2015/11/11