From patchwork Tue Mar 24 20:03:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 454020 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 0589E140119 for ; Wed, 25 Mar 2015 07:14:50 +1100 (AEDT) Received: from localhost ([::1]:34267 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaVDs-0000K2-56 for incoming@patchwork.ozlabs.org; Tue, 24 Mar 2015 16:14:48 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43716) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaV3n-0000b6-OY for qemu-devel@nongnu.org; Tue, 24 Mar 2015 16:04:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaV3h-00048g-Or for qemu-devel@nongnu.org; Tue, 24 Mar 2015 16:04:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37473) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaV3h-00048P-I8 for qemu-devel@nongnu.org; Tue, 24 Mar 2015 16:04:17 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 382348E917; Tue, 24 Mar 2015 20:04:17 +0000 (UTC) Received: from red.redhat.com (ovpn-113-74.phx2.redhat.com [10.3.113.74]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2OK3x7k028300; Tue, 24 Mar 2015 16:04:16 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Tue, 24 Mar 2015 14:03:48 -0600 Message-Id: <1427227433-5030-24-git-send-email-eblake@redhat.com> In-Reply-To: <1427227433-5030-1-git-send-email-eblake@redhat.com> References: <1427227433-5030-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, famz@redhat.com, armbru@redhat.com, wenchaoqemu@gmail.com, lcapitulino@redhat.com Subject: [Qemu-devel] [PATCH v5 23/28] qapi: More rigorous checking for type safety bypass 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 Now that we have a way to validate every type, we can also be stricter about enforcing that callers that want to bypass type safety in generated code. Prior to this patch, it didn't matter what value was associated with the key 'gen', but it looked odd that 'gen':'yes' could result in bypassing the generated code. These changes also enforce the changes made earlier in the series for documentation and consolidation of using '**' as the wildcard type, as well as 'gen':false as the canonical spelling for requesting type bypass. Signed-off-by: Eric Blake Reviewed-by: Markus Armbruster --- scripts/qapi.py | 23 ++++++++++++++++++----- tests/qapi-schema/type-bypass-bad-gen.err | 1 + tests/qapi-schema/type-bypass-bad-gen.exit | 2 +- tests/qapi-schema/type-bypass-bad-gen.json | 2 +- tests/qapi-schema/type-bypass-bad-gen.out | 3 --- tests/qapi-schema/type-bypass-no-gen.err | 1 + tests/qapi-schema/type-bypass-no-gen.exit | 2 +- tests/qapi-schema/type-bypass-no-gen.json | 2 +- tests/qapi-schema/type-bypass-no-gen.out | 3 --- 9 files changed, 24 insertions(+), 15 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 9421431..800e8e4 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -321,13 +321,14 @@ def check_name(expr_info, source, name, allow_optional = False): "%s uses invalid name '%s'" % (source, name)) def check_type(expr_info, source, data, allow_array = False, - allowed_metas = [], allow_dict = True, allow_optional = False): + allowed_metas = [], allow_dict = True, allow_optional = False, + allow_star = False): global all_names if data is None: return - if data == '**': + if allow_star and data == '**': return # Check if array type for data is okay @@ -343,6 +344,10 @@ def check_type(expr_info, source, data, allow_array = False, # Check if type name for data is okay if isinstance(data, str): + if data == '**': + raise QAPIExprError(expr_info, + "%s uses '**' but did not request 'gen':false" + % source) if not data in all_names: raise QAPIExprError(expr_info, "%s uses unknown type '%s'" @@ -367,19 +372,23 @@ def check_type(expr_info, source, data, allow_array = False, allow_array=True, allowed_metas=['built-in', 'union', 'alternate', 'struct', 'enum'], - allow_dict=True, allow_optional=True) + allow_dict=True, allow_optional=True, allow_star=allow_star) def check_command(expr, expr_info): name = expr['command'] + allow_star = expr.has_key('gen') + check_type(expr_info, "'data' for command '%s'" % name, expr.get('data'), - allowed_metas=['union', 'struct'], allow_optional=True) + allowed_metas=['union', 'struct'], allow_optional=True, + allow_star=allow_star) returns_meta = ['union', 'struct'] if name in returns_whitelist: returns_meta += ['built-in', 'alternate', 'enum'] check_type(expr_info, "'returns' for command '%s'" % name, expr.get('returns'), allow_array=True, - allowed_metas=returns_meta, allow_optional=True) + allowed_metas=returns_meta, allow_optional=True, + allow_star=allow_star) def check_event(expr, expr_info): global events @@ -574,6 +583,10 @@ def check_keys(expr_elem, meta, required, optional=[]): raise QAPIExprError(info, "%s '%s' has unknown key '%s'" % (meta, name, key)) + if (key == 'gen' or key == 'success-response') and value != False: + raise QAPIExprError(info, + "'%s' of %s '%s' should only use false value" + % (key, meta, name)) for key in required: if not expr.has_key(key): raise QAPIExprError(info, diff --git a/tests/qapi-schema/type-bypass-bad-gen.err b/tests/qapi-schema/type-bypass-bad-gen.err index e69de29..a83c3c6 100644 --- a/tests/qapi-schema/type-bypass-bad-gen.err +++ b/tests/qapi-schema/type-bypass-bad-gen.err @@ -0,0 +1 @@ +tests/qapi-schema/type-bypass-bad-gen.json:2: 'gen' of command 'foo' should only use false value diff --git a/tests/qapi-schema/type-bypass-bad-gen.exit b/tests/qapi-schema/type-bypass-bad-gen.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/type-bypass-bad-gen.exit +++ b/tests/qapi-schema/type-bypass-bad-gen.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/type-bypass-bad-gen.json b/tests/qapi-schema/type-bypass-bad-gen.json index bb70bee..e8dec34 100644 --- a/tests/qapi-schema/type-bypass-bad-gen.json +++ b/tests/qapi-schema/type-bypass-bad-gen.json @@ -1,2 +1,2 @@ -# FIXME: 'gen' should only appear with value false +# 'gen' should only appear with value false { 'command': 'foo', 'gen': 'whatever' } diff --git a/tests/qapi-schema/type-bypass-bad-gen.out b/tests/qapi-schema/type-bypass-bad-gen.out index e678f2c..e69de29 100644 --- a/tests/qapi-schema/type-bypass-bad-gen.out +++ b/tests/qapi-schema/type-bypass-bad-gen.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'foo'), ('gen', 'whatever')])] -[] -[] diff --git a/tests/qapi-schema/type-bypass-no-gen.err b/tests/qapi-schema/type-bypass-no-gen.err index e69de29..20cef0a 100644 --- a/tests/qapi-schema/type-bypass-no-gen.err +++ b/tests/qapi-schema/type-bypass-no-gen.err @@ -0,0 +1 @@ +tests/qapi-schema/type-bypass-no-gen.json:2: Member 'arg' of 'data' for command 'unsafe' uses '**' but did not request 'gen':false diff --git a/tests/qapi-schema/type-bypass-no-gen.exit b/tests/qapi-schema/type-bypass-no-gen.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/type-bypass-no-gen.exit +++ b/tests/qapi-schema/type-bypass-no-gen.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/type-bypass-no-gen.json b/tests/qapi-schema/type-bypass-no-gen.json index af87c19..49b5742 100644 --- a/tests/qapi-schema/type-bypass-no-gen.json +++ b/tests/qapi-schema/type-bypass-no-gen.json @@ -1,2 +1,2 @@ -# FIXME: type bypass should only work with 'gen':false +# type bypass only works with 'gen':'no' { 'command': 'unsafe', 'data': { 'arg': '**' }, 'returns': '**' } diff --git a/tests/qapi-schema/type-bypass-no-gen.out b/tests/qapi-schema/type-bypass-no-gen.out index 8b2a9ac..e69de29 100644 --- a/tests/qapi-schema/type-bypass-no-gen.out +++ b/tests/qapi-schema/type-bypass-no-gen.out @@ -1,3 +0,0 @@ -[OrderedDict([('command', 'unsafe'), ('data', OrderedDict([('arg', '**')])), ('returns', '**')])] -[] -[]