From patchwork Sun Apr 5 04:07:58 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 458212 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 6E2B01400A0 for ; Sun, 5 Apr 2015 14:21:57 +1000 (AEST) Received: from localhost ([::1]:35097 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yec4J-0006SG-BX for incoming@patchwork.ozlabs.org; Sun, 05 Apr 2015 00:21:55 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42956) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YebrL-0000OY-Nu for qemu-devel@nongnu.org; Sun, 05 Apr 2015 00:08:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YebrJ-0002tl-W3 for qemu-devel@nongnu.org; Sun, 05 Apr 2015 00:08:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54595) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YebrJ-0002th-Nt for qemu-devel@nongnu.org; Sun, 05 Apr 2015 00:08:29 -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 (8.14.4/8.14.4) with ESMTP id t3548Th1026835 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Sun, 5 Apr 2015 00:08:29 -0400 Received: from red.redhat.com (ovpn-113-46.phx2.redhat.com [10.3.113.46]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t35489wN008523; Sun, 5 Apr 2015 00:08:28 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Sat, 4 Apr 2015 22:07:58 -0600 Message-Id: <1428206887-7921-28-git-send-email-eblake@redhat.com> In-Reply-To: <1428206887-7921-1-git-send-email-eblake@redhat.com> References: <1428206887-7921-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, berto@igalia.co, armbru@redhat.com Subject: [Qemu-devel] [PATCH v6 27/36] 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. Note that 'gen':false is a one-way switch away from the default; we do not support 'gen':true (similar for 'success-response). In practice, this doesn't matter. Signed-off-by: Eric Blake Reviewed-by: Markus Armbruster --- v6: rebase onto earlier changes; enhance commit message --- scripts/qapi.py | 22 +++++++++++++++++----- 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, 23 insertions(+), 15 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 963f088..8c83de0 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -324,14 +324,15 @@ def check_name(expr_info, source, name, allow_optional = False, "%s uses invalid name '%s'" % (source, name)) def check_type(expr_info, source, value, allow_array = False, - allow_dict = False, allow_optional = False, allow_metas = []): + allow_dict = False, allow_optional = False, + allow_star = False, allow_metas = []): global all_names orig_value = value if value is None: return - if value == '**': + if allow_star and value == '**': return # Check if array type for value is okay @@ -348,6 +349,10 @@ def check_type(expr_info, source, value, allow_array = False, # Check if type name for value is okay if isinstance(value, str): + if value == '**': + raise QAPIExprError(expr_info, + "%s uses '**' but did not request 'gen':false" + % source) if not value in all_names: raise QAPIExprError(expr_info, "%s uses unknown type '%s'" @@ -371,19 +376,22 @@ def check_type(expr_info, source, value, allow_array = False, check_type(expr_info, "Member '%s' of %s" % (key, source), arg, allow_array=True, allow_dict=True, allow_optional=True, allow_metas=['built-in', 'union', 'alternate', 'struct', - 'enum']) + 'enum'], 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'), allow_dict=True, allow_optional=True, - allow_metas=['union', 'struct']) + allow_metas=['union', 'struct'], 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, allow_dict=True, - allow_optional=True, allow_metas=returns_meta) + allow_optional=True, allow_metas=returns_meta, + allow_star=allow_star) def check_event(expr, expr_info): global events @@ -579,6 +587,10 @@ def check_keys(expr_elem, meta, required, optional=[]): raise QAPIExprError(info, "Unknown key '%s' in %s '%s'" % (key, meta, name)) + 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..4feae37 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':false { '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', '**')])] -[] -[]