From patchwork Mon Sep 21 21:57:30 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 520606 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BF2D11401AF for ; Tue, 22 Sep 2015 08:01:53 +1000 (AEST) Received: from localhost ([::1]:34202 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze99j-0004nj-Jy for incoming@patchwork.ozlabs.org; Mon, 21 Sep 2015 18:01:51 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60424) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze96I-0007dx-IQ for qemu-devel@nongnu.org; Mon, 21 Sep 2015 17:58:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ze96G-0000qi-Qd for qemu-devel@nongnu.org; Mon, 21 Sep 2015 17:58:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46328) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ze96G-0000qZ-IG for qemu-devel@nongnu.org; Mon, 21 Sep 2015 17:58:16 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 33C66385A6A; Mon, 21 Sep 2015 21:58:16 +0000 (UTC) Received: from red.redhat.com (ovpn-113-166.phx2.redhat.com [10.3.113.166]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8LLw4ba027229; Mon, 21 Sep 2015 17:58:15 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 21 Sep 2015 15:57:30 -0600 Message-Id: <1442872682-6523-15-git-send-email-eblake@redhat.com> In-Reply-To: <1442872682-6523-1-git-send-email-eblake@redhat.com> References: <1442872682-6523-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Michael Roth , marcandre.lureau@redhat.com, DirtY.iCE.hu@gmail.com, armbru@redhat.com, ehabkost@redhat.com Subject: [Qemu-devel] [PATCH v5 14/46] qapi: Detect collisions in C member names X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Detect attempts to declare two object members that would result in the same C member name, by keying the 'seen' dictionary off of the C name rather than the qapi name. As this is the first error raised within the QapiSchema check() methods, we also have to pass 'info' around through the call stack, and fix the overall 'try' to check for errors detected during the check() phase. The resulting error messages demonstrate the utility of the .describe() method added previously. Signed-off-by: Eric Blake --- scripts/qapi.py | 44 +++++++++++++++---------- tests/qapi-schema/args-name-clash.err | 1 + tests/qapi-schema/args-name-clash.exit | 2 +- tests/qapi-schema/args-name-clash.json | 2 +- tests/qapi-schema/args-name-clash.out | 6 ---- tests/qapi-schema/flat-union-branch-clash2.err | 1 + tests/qapi-schema/flat-union-branch-clash2.exit | 2 +- tests/qapi-schema/flat-union-branch-clash2.json | 2 +- tests/qapi-schema/flat-union-branch-clash2.out | 14 -------- 9 files changed, 33 insertions(+), 41 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 6bc13f1..0a0ac90 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -103,6 +103,7 @@ class QAPISchemaError(Exception): class QAPIExprError(Exception): def __init__(self, expr_info, msg): Exception.__init__(self) + assert expr_info self.info = expr_info self.msg = msg @@ -956,11 +957,12 @@ class QAPISchemaObjectType(QAPISchemaType): members = [] seen = {} for m in members: - seen[m.name] = m + assert m.c_name() not in seen + seen[m.c_name()] = m for m in self.local_members: - m.check(schema, members, seen) + m.check(schema, self._info, members, seen) if self.variants: - self.variants.check(schema, members, seen) + self.variants.check(schema, self._info, members, seen) self.members = members def is_implicit(self): @@ -997,12 +999,19 @@ class QAPISchemaObjectTypeMember(object): self.optional = optional self._owner = owner - def check(self, schema, all_members, seen): - assert self.name not in seen + def check(self, schema, info, all_members, seen): + if self.c_name() in seen: + raise QAPIExprError(info, + "%s collides with %s" + % (self.describe(), + seen[self.c_name()].describe())) self.type = schema.lookup_type(self._type_name) assert self.type all_members.append(self) - seen[self.name] = self + seen[self.c_name()] = self + + def c_name(self): + return c_name(self.name) def describe(self): return "'%s' (member of %s)" % (self.name, self._owner) @@ -1024,23 +1033,24 @@ class QAPISchemaObjectTypeVariants(object): '(implicit)') self.variants = variants - def check(self, schema, members, seen): + def check(self, schema, info, members, seen): if self.tag_name: - self.tag_member = seen[self.tag_name] + self.tag_member = seen[c_name(self.tag_name)] + assert self.tag_name == self.tag_member.name else: - self.tag_member.check(schema, members, seen) + self.tag_member.check(schema, info, members, seen) assert isinstance(self.tag_member.type, QAPISchemaEnumType) for v in self.variants: vseen = dict(seen) - v.check(schema, self.tag_member.type, vseen) + v.check(schema, info, self.tag_member.type, vseen) class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): def __init__(self, name, typ, owner): QAPISchemaObjectTypeMember.__init__(self, name, typ, False, owner) - def check(self, schema, tag_type, seen): - QAPISchemaObjectTypeMember.check(self, schema, [], seen) + def check(self, schema, info, tag_type, seen): + QAPISchemaObjectTypeMember.check(self, schema, info, [], seen) assert self.name in tag_type.values def describe(self): @@ -1065,7 +1075,7 @@ class QAPISchemaAlternateType(QAPISchemaType): self.variants = variants def check(self, schema): - self.variants.check(schema, [], {}) + self.variants.check(schema, self._info, [], {}) def json_type(self): return 'value' @@ -1122,13 +1132,13 @@ class QAPISchema(object): def __init__(self, fname): try: self.exprs = check_exprs(QAPISchemaParser(open(fname, "r")).exprs) + self._entity_dict = {} + self._def_predefineds() + self._def_exprs() + self.check() except (QAPISchemaError, QAPIExprError), err: print >>sys.stderr, err exit(1) - self._entity_dict = {} - self._def_predefineds() - self._def_exprs() - self.check() def _def_entity(self, ent): assert ent.name not in self._entity_dict diff --git a/tests/qapi-schema/args-name-clash.err b/tests/qapi-schema/args-name-clash.err index e69de29..743afdb 100644 --- a/tests/qapi-schema/args-name-clash.err +++ b/tests/qapi-schema/args-name-clash.err @@ -0,0 +1 @@ +tests/qapi-schema/args-name-clash.json:2: 'a_b' (member of oops arguments) collides with 'a-b' (member of oops arguments) diff --git a/tests/qapi-schema/args-name-clash.exit b/tests/qapi-schema/args-name-clash.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/args-name-clash.exit +++ b/tests/qapi-schema/args-name-clash.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/args-name-clash.json b/tests/qapi-schema/args-name-clash.json index 19bf792..602db6a 100644 --- a/tests/qapi-schema/args-name-clash.json +++ b/tests/qapi-schema/args-name-clash.json @@ -1,2 +1,2 @@ -# FIXME - we should reject data with members that clash when mapped to C names +# we reject data with members that clash when mapped to C names { 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } } diff --git a/tests/qapi-schema/args-name-clash.out b/tests/qapi-schema/args-name-clash.out index 9b2f6e4..e69de29 100644 --- a/tests/qapi-schema/args-name-clash.out +++ b/tests/qapi-schema/args-name-clash.out @@ -1,6 +0,0 @@ -object :empty -object :obj-oops-arg - member a-b: str optional=False - member a_b: str optional=False -command oops :obj-oops-arg -> None - gen=True success_response=True diff --git a/tests/qapi-schema/flat-union-branch-clash2.err b/tests/qapi-schema/flat-union-branch-clash2.err index e69de29..99eacd2 100644 --- a/tests/qapi-schema/flat-union-branch-clash2.err +++ b/tests/qapi-schema/flat-union-branch-clash2.err @@ -0,0 +1 @@ +tests/qapi-schema/flat-union-branch-clash2.json:10: 'c-d' (branch of TestUnion)' collides with 'c_d' (member of Base) diff --git a/tests/qapi-schema/flat-union-branch-clash2.exit b/tests/qapi-schema/flat-union-branch-clash2.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/flat-union-branch-clash2.exit +++ b/tests/qapi-schema/flat-union-branch-clash2.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/flat-union-branch-clash2.json b/tests/qapi-schema/flat-union-branch-clash2.json index b3eefb3..b0dd85e 100644 --- a/tests/qapi-schema/flat-union-branch-clash2.json +++ b/tests/qapi-schema/flat-union-branch-clash2.json @@ -1,4 +1,4 @@ -# FIXME: we should check for no duplicate C names between branches and base +# we check for no duplicate C names between branches and base { 'enum': 'TestEnum', 'data': [ 'base', 'c-d' ] } { 'struct': 'Base', diff --git a/tests/qapi-schema/flat-union-branch-clash2.out b/tests/qapi-schema/flat-union-branch-clash2.out index 8e0da73..e69de29 100644 --- a/tests/qapi-schema/flat-union-branch-clash2.out +++ b/tests/qapi-schema/flat-union-branch-clash2.out @@ -1,14 +0,0 @@ -object :empty -object Base - member enum1: TestEnum optional=False - member c_d: str optional=True -object Branch1 - member string: str optional=False -object Branch2 - member value: int optional=False -enum TestEnum ['base', 'c-d'] -object TestUnion - base Base - tag enum1 - case base: Branch1 - case c-d: Branch2