From patchwork Fri Sep 19 22:24:53 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 391430 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 63AB0140161 for ; Sat, 20 Sep 2014 08:29:58 +1000 (EST) Received: from localhost ([::1]:60841 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XV6ge-00046R-ES for incoming@patchwork.ozlabs.org; Fri, 19 Sep 2014 18:29:56 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43293) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XV6cW-0004PR-Uu for qemu-devel@nongnu.org; Fri, 19 Sep 2014 18:25:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XV6cP-0004Yq-TM for qemu-devel@nongnu.org; Fri, 19 Sep 2014 18:25:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41728) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XV6cP-0004WY-Hf for qemu-devel@nongnu.org; Fri, 19 Sep 2014 18:25:33 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8JMPQrh025112 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 19 Sep 2014 18:25:27 -0400 Received: from red.redhat.com (ovpn-113-164.phx2.redhat.com [10.3.113.164]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8JMPLPb007538; Fri, 19 Sep 2014 18:25:26 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 19 Sep 2014 16:24:53 -0600 Message-Id: <1411165504-18198-9-git-send-email-eblake@redhat.com> In-Reply-To: <1411165504-18198-1-git-send-email-eblake@redhat.com> References: <1411165504-18198-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Luiz Capitulino , Fam Zheng , Markus Armbruster , wenchaoqemu@gmail.com Subject: [Qemu-devel] [PATCH v4 08/19] qapi: Better error messages for bad expressions 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 The previous commit demonstrated that the generator overlooked some fairly basic broken expressions: - missing metataype - metatype key has a non-string value - unknown key in relation to the metatype - conflicting metatype (this patch treats the second metatype as an unknown key of the first key visited, which is not necessarily the first key the user typed) Add check_keys to cover these situations, and update testcases to match. A couple other tests (enum-missing-data, indented-expr) had to change since the validation added here occurs so early. While valid .json files won't trigger any of these cases, we might as well be nicer to developers that make a typo while trying to add new QAPI code. Signed-off-by: Eric Blake --- scripts/qapi.py | 76 +++++++++++++++++++++++++-------- tests/qapi-schema/bad-type-dict.err | 1 + tests/qapi-schema/bad-type-dict.exit | 2 +- tests/qapi-schema/bad-type-dict.json | 2 +- tests/qapi-schema/bad-type-dict.out | 3 -- tests/qapi-schema/double-type.err | 1 + tests/qapi-schema/double-type.exit | 2 +- tests/qapi-schema/double-type.json | 2 +- tests/qapi-schema/double-type.out | 3 -- tests/qapi-schema/enum-missing-data.err | 2 +- tests/qapi-schema/indented-expr.json | 4 +- tests/qapi-schema/indented-expr.out | 2 +- tests/qapi-schema/missing-type.err | 1 + tests/qapi-schema/missing-type.exit | 2 +- tests/qapi-schema/missing-type.json | 2 +- tests/qapi-schema/missing-type.out | 3 -- tests/qapi-schema/unknown-expr-key.err | 1 + tests/qapi-schema/unknown-expr-key.exit | 2 +- tests/qapi-schema/unknown-expr-key.json | 2 +- tests/qapi-schema/unknown-expr-key.out | 3 -- 20 files changed, 75 insertions(+), 41 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 85aa8bf..8fbc45f 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -348,7 +348,29 @@ def check_exprs(schema): if expr.has_key('enum'): check_enum(expr, info) +def check_keys(expr_elem, meta, required, optional=[]): + expr = expr_elem['expr'] + info = expr_elem['info'] + name = expr[meta] + if not isinstance(name, basestring): + raise QAPIExprError(info, + "%s key must have a string value" % meta) + expr_elem['name'] = name + required.append(meta) + for (key, value) in expr.items(): + if not key in required and not key in optional: + raise QAPIExprError(info, + "%s '%s' has unknown key '%s'" + % (meta, name, key)) + for key in required: + if not expr.has_key(key): + raise QAPIExprError(info, + "%s '%s' is missing key '%s'" + % (meta, name, key)) + + def parse_schema(input_file): + # First pass: read entire file into memory try: schema = QAPISchema(open(input_file, "r")) except (QAPISchemaError, QAPIExprError), e: @@ -357,24 +379,44 @@ def parse_schema(input_file): exprs = [] - for expr_elem in schema.exprs: - expr = expr_elem['expr'] - if expr.has_key('enum'): - add_enum(expr['enum'], expr.get('data')) - elif expr.has_key('union'): - add_union(expr) - elif expr.has_key('type'): - add_struct(expr) - exprs.append(expr) - - # Try again for hidden UnionKind enum - for expr_elem in schema.exprs: - expr = expr_elem['expr'] - if expr.has_key('union'): - if not discriminator_find_enum_define(expr): - add_enum('%sKind' % expr['union']) - try: + # Next pass: learn the types and check for valid expression keys. At + # this point, top-level 'include' has already been flattened. + for expr_elem in schema.exprs: + expr = expr_elem['expr'] + if expr.has_key('enum'): + check_keys(expr_elem, 'enum', ['data']) + add_enum(expr['enum'], expr['data']) + elif expr.has_key('union'): + # Two styles of union, based on discriminator + discriminator = expr.get('discriminator') + if discriminator == {}: + check_keys(expr_elem, 'union', ['data', 'discriminator']) + else: + check_keys(expr_elem, 'union', ['data'], + ['base', 'discriminator']) + add_union(expr) + elif expr.has_key('type'): + check_keys(expr_elem, 'type', ['data'], ['base']) + add_struct(expr) + elif expr.has_key('command'): + check_keys(expr_elem, 'command', [], + ['data', 'returns', 'gen', 'success-response']) + elif expr.has_key('event'): + check_keys(expr_elem, 'event', [], ['data']) + else: + raise QAPIExprError(expr_elem['info'], + "expression is missing metatype") + exprs.append(expr) + + # Try again for hidden UnionKind enum + for expr_elem in schema.exprs: + expr = expr_elem['expr'] + if expr.has_key('union'): + if not discriminator_find_enum_define(expr): + add_enum('%sKind' % expr['union']) + + # Final pass - validate that exprs make sense check_exprs(schema) except QAPIExprError, e: print >>sys.stderr, e diff --git a/tests/qapi-schema/bad-type-dict.err b/tests/qapi-schema/bad-type-dict.err index e69de29..b7c73cc 100644 --- a/tests/qapi-schema/bad-type-dict.err +++ b/tests/qapi-schema/bad-type-dict.err @@ -0,0 +1 @@ +tests/qapi-schema/bad-type-dict.json:2: command key must have a string value diff --git a/tests/qapi-schema/bad-type-dict.exit b/tests/qapi-schema/bad-type-dict.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/bad-type-dict.exit +++ b/tests/qapi-schema/bad-type-dict.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/bad-type-dict.json b/tests/qapi-schema/bad-type-dict.json index 3c392a7..2a91b24 100644 --- a/tests/qapi-schema/bad-type-dict.json +++ b/tests/qapi-schema/bad-type-dict.json @@ -1,2 +1,2 @@ -# FIXME: we should reject an expression with a metatype that is not a string +# we reject an expression with a metatype that is not a string { 'command': { } } diff --git a/tests/qapi-schema/bad-type-dict.out b/tests/qapi-schema/bad-type-dict.out index c62f1ed..e69de29 100644 --- a/tests/qapi-schema/bad-type-dict.out +++ b/tests/qapi-schema/bad-type-dict.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', OrderedDict())])] -[] -[] diff --git a/tests/qapi-schema/double-type.err b/tests/qapi-schema/double-type.err index e69de29..394127b 100644 --- a/tests/qapi-schema/double-type.err +++ b/tests/qapi-schema/double-type.err @@ -0,0 +1 @@ +tests/qapi-schema/double-type.json:2: type 'bar' has unknown key 'command' diff --git a/tests/qapi-schema/double-type.exit b/tests/qapi-schema/double-type.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/double-type.exit +++ b/tests/qapi-schema/double-type.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/double-type.json b/tests/qapi-schema/double-type.json index 6ca96b9..471623a 100644 --- a/tests/qapi-schema/double-type.json +++ b/tests/qapi-schema/double-type.json @@ -1,2 +1,2 @@ -# FIXME: we should reject an expression with ambiguous metatype +# we reject an expression with ambiguous metatype { 'command': 'foo', 'type': 'bar', 'data': { } } diff --git a/tests/qapi-schema/double-type.out b/tests/qapi-schema/double-type.out index 3e244f5..e69de29 100644 --- a/tests/qapi-schema/double-type.out +++ b/tests/qapi-schema/double-type.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'foo'), ('type', 'bar'), ('data', OrderedDict())])] -[] -[OrderedDict([('command', 'foo'), ('type', 'bar'), ('data', OrderedDict())])] diff --git a/tests/qapi-schema/enum-missing-data.err b/tests/qapi-schema/enum-missing-data.err index 4b8b59e..62f4a7f 100644 --- a/tests/qapi-schema/enum-missing-data.err +++ b/tests/qapi-schema/enum-missing-data.err @@ -1 +1 @@ -tests/qapi-schema/enum-missing-data.json:2: enum 'MyEnum' requires an array for 'data' +tests/qapi-schema/enum-missing-data.json:2: enum 'MyEnum' is missing key 'data' diff --git a/tests/qapi-schema/indented-expr.json b/tests/qapi-schema/indented-expr.json index d80af60..7115d31 100644 --- a/tests/qapi-schema/indented-expr.json +++ b/tests/qapi-schema/indented-expr.json @@ -1,2 +1,2 @@ -{ 'id' : 'eins' } - { 'id' : 'zwei' } +{ 'command' : 'eins' } + { 'command' : 'zwei' } diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out index 98af89a..b5ce915 100644 --- a/tests/qapi-schema/indented-expr.out +++ b/tests/qapi-schema/indented-expr.out @@ -1,3 +1,3 @@ -[OrderedDict([('id', 'eins')]), OrderedDict([('id', 'zwei')])] +[OrderedDict([('command', 'eins')]), OrderedDict([('command', 'zwei')])] [] [] diff --git a/tests/qapi-schema/missing-type.err b/tests/qapi-schema/missing-type.err index e69de29..19b7c49 100644 --- a/tests/qapi-schema/missing-type.err +++ b/tests/qapi-schema/missing-type.err @@ -0,0 +1 @@ +tests/qapi-schema/missing-type.json:2: expression is missing metatype diff --git a/tests/qapi-schema/missing-type.exit b/tests/qapi-schema/missing-type.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/missing-type.exit +++ b/tests/qapi-schema/missing-type.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/missing-type.json b/tests/qapi-schema/missing-type.json index 1696f5c..ff5349d 100644 --- a/tests/qapi-schema/missing-type.json +++ b/tests/qapi-schema/missing-type.json @@ -1,2 +1,2 @@ -# FIXME: we should reject an expression with missing metatype +# we reject an expression with missing metatype { 'data': { } } diff --git a/tests/qapi-schema/missing-type.out b/tests/qapi-schema/missing-type.out index 67fd4fa..e69de29 100644 --- a/tests/qapi-schema/missing-type.out +++ b/tests/qapi-schema/missing-type.out @@ -1,3 +0,0 @@ -[OrderedDict([('data', OrderedDict())])] -[] -[] diff --git a/tests/qapi-schema/unknown-expr-key.err b/tests/qapi-schema/unknown-expr-key.err index e69de29..3b35508 100644 --- a/tests/qapi-schema/unknown-expr-key.err +++ b/tests/qapi-schema/unknown-expr-key.err @@ -0,0 +1 @@ +tests/qapi-schema/unknown-expr-key.json:2: type 'bar' has unknown key 'bogus' diff --git a/tests/qapi-schema/unknown-expr-key.exit b/tests/qapi-schema/unknown-expr-key.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/unknown-expr-key.exit +++ b/tests/qapi-schema/unknown-expr-key.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/unknown-expr-key.json b/tests/qapi-schema/unknown-expr-key.json index 1e9282d..ba7bdf3 100644 --- a/tests/qapi-schema/unknown-expr-key.json +++ b/tests/qapi-schema/unknown-expr-key.json @@ -1,2 +1,2 @@ -# FIXME: we should reject an expression with unknown top-level keys +# we reject an expression with unknown top-level keys { 'type': 'bar', 'data': { 'string': 'str'}, 'bogus': { } } diff --git a/tests/qapi-schema/unknown-expr-key.out b/tests/qapi-schema/unknown-expr-key.out index c93f020..e69de29 100644 --- a/tests/qapi-schema/unknown-expr-key.out +++ b/tests/qapi-schema/unknown-expr-key.out @@ -1,3 +0,0 @@ -[OrderedDict([('type', 'bar'), ('data', OrderedDict([('string', 'str')])), ('bogus', OrderedDict())])] -[] -[OrderedDict([('type', 'bar'), ('data', OrderedDict([('string', 'str')])), ('bogus', OrderedDict())])]