From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: armbru@redhat.com, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: [Qemu-devel] [PATCH v16 08/24] qapi-commands: Wrap argument visit in visit_start_struct
Date: Thu, 28 Apr 2016 15:45:16 -0600 [thread overview]
Message-ID: <1461879932-9020-9-git-send-email-eblake@redhat.com> (raw)
In-Reply-To: <1461879932-9020-1-git-send-email-eblake@redhat.com>
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 <eblake@redhat.com>
---
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);
}
--
2.5.5
next prev parent reply other threads:[~2016-04-28 21:45 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-28 21:45 [Qemu-devel] [PATCH v16 00/24] qapi visitor cleanups (post-introspection cleanups subset E) Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 01/24] qapi-visit: Add visitor.type classification Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 02/24] qapi: Guarantee NULL obj on input visitor callback error Eric Blake
2016-04-29 8:28 ` Markus Armbruster
2016-04-29 12:10 ` Eric Blake
2016-04-29 12:17 ` Eric Blake
2016-04-29 12:59 ` Markus Armbruster
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 03/24] qmp: Drop dead command->type Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 04/24] qmp-input: Clean up stack handling Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 05/24] qapi: Consolidate QMP input visitor creation Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 06/24] qapi: Use strict QMP input visitor in more places Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 07/24] qmp-input: Don't consume input when checking has_member Eric Blake
2016-04-28 21:45 ` Eric Blake [this message]
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 09/24] qom: Wrap prop visit in visit_start_struct Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 10/24] qmp-input: Require struct push to visit members of top dict Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 11/24] qmp-input: Refactor when list is advanced Eric Blake
2016-04-29 8:50 ` Markus Armbruster
2016-04-29 12:15 ` Eric Blake
2016-04-29 13:03 ` Markus Armbruster
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 12/24] qapi: Document visitor interfaces, add assertions Eric Blake
2016-05-04 14:05 ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2016-05-04 14:49 ` Eric Blake
2016-05-04 15:04 ` Markus Armbruster
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 13/24] tests: Add check-qnull Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 14/24] qapi: Add visit_type_null() visitor Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 15/24] qmp: Support explicit null during visits Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 16/24] spapr_drc: Expose 'null' in qom-get when there is no fdt Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 17/24] qmp: Add qmp_output_visitor_reset() Eric Blake
2016-05-10 4:20 ` [Qemu-devel] [PATCH v16A 17/24] qmp: Don't reuse qmp visitor after grabbing output Eric Blake
2016-05-10 8:18 ` Markus Armbruster
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 18/24] qmp: Tighten output visitor rules Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 19/24] qapi: Split visit_end_struct() into pieces Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 20/24] qapi: Don't pass NULL to printf in string input visitor Eric Blake
2016-04-29 9:03 ` Markus Armbruster
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 21/24] tests/string-input-visitor: Add negative integer tests Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 22/24] qapi: Fix string input visitor handling of invalid list Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 23/24] qapi: Simplify semantics of visit_next_list() Eric Blake
2016-04-28 21:45 ` [Qemu-devel] [PATCH v16 24/24] qapi: Change visit_type_FOO() to no longer return partial objects Eric Blake
2016-04-29 11:13 ` [Qemu-devel] [PATCH v16 00/24] qapi visitor cleanups (post-introspection cleanups subset E) Markus Armbruster
2016-04-29 12:16 ` Eric Blake
2016-04-29 13:09 ` Markus Armbruster
2016-04-29 14:09 ` Eric Blake
2016-05-04 13:54 ` Markus Armbruster
2016-05-04 14:07 ` Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1461879932-9020-9-git-send-email-eblake@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).