From patchwork Thu Apr 28 21:45:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 616455 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qwr4d5SRQz9t3p for ; Fri, 29 Apr 2016 07:46:25 +1000 (AEST) Received: from localhost ([::1]:51158 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avtlP-0000kf-JQ for incoming@patchwork.ozlabs.org; Thu, 28 Apr 2016 17:46:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55002) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avtki-0007m9-8i for qemu-devel@nongnu.org; Thu, 28 Apr 2016 17:45:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avtkh-0000Dr-55 for qemu-devel@nongnu.org; Thu, 28 Apr 2016 17:45:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38683) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avtkg-0000DX-UU for qemu-devel@nongnu.org; Thu, 28 Apr 2016 17:45:39 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 843791602B9; Thu, 28 Apr 2016 21:45:38 +0000 (UTC) Received: from red.redhat.com (ovpn-113-21.phx2.redhat.com [10.3.113.21]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3SLjY8U019512; Thu, 28 Apr 2016 17:45:38 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Thu, 28 Apr 2016 15:45:16 -0600 Message-Id: <1461879932-9020-9-git-send-email-eblake@redhat.com> In-Reply-To: <1461879932-9020-1-git-send-email-eblake@redhat.com> References: <1461879932-9020-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 28 Apr 2016 21:45:38 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v16 08/24] qapi-commands: Wrap argument visit in visit_start_struct X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: armbru@redhat.com, Michael Roth Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" The qmp-input visitor was allowing callers to play rather fast and loose: when visiting a QDict, you could grab members of the root dictionary without first pushing into the dict; among the culprit callers was the generated marshal code on the 'arguments' dictionary of a QMP command. But we are about to tighten the input visitor, at which point the generated marshal code MUST follow the same paradigms as everyone else, of pushing into the struct before grabbing its keys. Generated code grows as follows: |@@ -515,7 +641,12 @@ void qmp_marshal_blockdev_backup(QDict * | BlockdevBackup arg = {0}; | | v = qmp_input_get_visitor(qiv); |+ visit_start_struct(v, NULL, NULL, 0, &err); |+ if (err) { |+ goto out; |+ } | visit_type_BlockdevBackup_members(v, &arg, &err); |+ visit_end_struct(v, err ? NULL : &err); | if (err) { | goto out; | } |@@ -527,7 +715,9 @@ out: | qmp_input_visitor_cleanup(qiv); | qdv = qapi_dealloc_visitor_new(); | v = qapi_dealloc_get_visitor(qdv); |+ visit_start_struct(v, NULL, NULL, 0, NULL); | visit_type_BlockdevBackup_members(v, &arg, NULL); |+ visit_end_struct(v, NULL); | qapi_dealloc_visitor_cleanup(qdv); | } The use of 'err ? NULL : &err' is temporary; a later patch will clean that up when it splits visit_end_struct(). Prior to this patch, the fact that there was no final visit_end_struct() meant that even though we are using a strict input visit, the marshalling code was not detecting excess input at the top level (only in nested levels). Fortunately, we have code in monitor.c:qmp_check_client_args() that also checks for no excess arguments at the top level. But as the generated code is more compact than the manual check, a later patch will clean up monitor.c to drop the redundancy added here. Signed-off-by: Eric Blake --- v15: hoist earlier in series, improve commit message, update docs v14: rebase to master context v13: rebase to earlier patches v12: new patch --- scripts/qapi-commands.py | 7 +++++++ docs/qapi-code-gen.txt | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 6261e44..04549fa 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -121,7 +121,12 @@ def gen_marshal(name, arg_type, ret_type): %(c_name)s arg = {0}; v = qmp_input_get_visitor(qiv); + visit_start_struct(v, NULL, NULL, 0, &err); + if (err) { + goto out; + } visit_type_%(c_name)s_members(v, &arg, &err); + visit_end_struct(v, err ? NULL : &err); if (err) { goto out; } @@ -150,7 +155,9 @@ out: qmp_input_visitor_cleanup(qiv); qdv = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(qdv); + visit_start_struct(v, NULL, NULL, 0, NULL); visit_type_%(c_name)s_members(v, &arg, NULL); + visit_end_struct(v, NULL); qapi_dealloc_visitor_cleanup(qdv); ''', c_name=arg_type.c_name()) diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index 4a917f9..b4ae1be 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -1002,7 +1002,12 @@ Example: UserDefOneList *arg1 = NULL; v = qmp_input_get_visitor(qiv); + visit_start_struct(v, NULL, NULL, 0, &err); + if (err) { + goto out; + } visit_type_UserDefOneList(v, "arg1", &arg1, &err); + visit_end_struct(v, err ? NULL : &err); if (err) { goto out; } @@ -1019,7 +1024,9 @@ Example: qmp_input_visitor_cleanup(qiv); qdv = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(qdv); + visit_start_struct(v, NULL, NULL, 0, NULL); visit_type_UserDefOneList(v, "arg1", &arg1, NULL); + visit_end_struct(v, NULL); qapi_dealloc_visitor_cleanup(qdv); }