From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44696) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zlr6x-00029Z-Vh for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:22:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zlr6w-0005r3-QS for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:22:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48008) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zlr6w-0005qR-IS for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:22:50 -0400 From: Eric Blake Date: Mon, 12 Oct 2015 22:22:38 -0600 Message-Id: <1444710158-8723-19-git-send-email-eblake@redhat.com> In-Reply-To: <1444710158-8723-1-git-send-email-eblake@redhat.com> References: <1444710158-8723-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v8 18/18] qapi: Detect base class loops List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: armbru@redhat.com, Michael Roth 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). Signed-off-by: Eric Blake --- v8: improve commit message 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 b21e38e..d6825ce 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -927,7 +927,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 443e345..1ef44aa 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -251,6 +251,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