[Qemu-devel] [PATCH v7 14/14] qapi: Detect base class loops

[Eric Blake]

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 about too deep of nesting).

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 unbounded recursion is platform-specific (no
guarantees at what limit it will quit trying to recurse) and
rather large for how little useful content it provides.

Signed-off-by: Eric Blake <address@hidden>

---
v7: improve commit message
v6: rebase to earlier info changes
---
 scripts/qapi.py                   | 6 +++++-
 tests/Makefile                    | 1 +
 tests/qapi-schema/base-cycle.err  | 1 +
 tests/qapi-schema/base-cycle.exit | 1 +
 tests/qapi-schema/base-cycle.json | 3 +++
 tests/qapi-schema/base-cycle.out  | 0
 6 files changed, 11 insertions(+), 1 deletion(-)
 create mode 100644 tests/qapi-schema/base-cycle.err
 create mode 100644 tests/qapi-schema/base-cycle.exit
 create mode 100644 tests/qapi-schema/base-cycle.json
 create mode 100644 tests/qapi-schema/base-cycle.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 55aca40..13deba5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -942,7 +942,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 b721a18..c11ee78 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -248,6 +248,7 @@ 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.json
 qapi-schema += command-int.json
 qapi-schema += comments.json
 qapi-schema += double-data.json
diff --git a/tests/qapi-schema/base-cycle.err b/tests/qapi-schema/base-cycle.err
new file mode 100644
index 0000000..e0221b5
--- /dev/null
+++ b/tests/qapi-schema/base-cycle.err
@@ -0,0 +1 @@
+tests/qapi-schema/base-cycle.json:2: Object Base1 cyclically depends on Base2
diff --git a/tests/qapi-schema/base-cycle.exit 
b/tests/qapi-schema/base-cycle.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/base-cycle.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/base-cycle.json 
b/tests/qapi-schema/base-cycle.json
new file mode 100644
index 0000000..2866772
--- /dev/null
+++ b/tests/qapi-schema/base-cycle.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.out b/tests/qapi-schema/base-cycle.out
new file mode 100644
index 0000000..e69de29
-- 
2.4.3