[PULL,08/23] qapi-commands: Wrap argument visit in visit_start_struct

Commit Message

Markus Armbruster May 12, 2016, 7:58 a.m. UTC

From: Eric Blake <[e-mail]>

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 <[e-mail]>
Message-Id: <[e-mail]>
Signed-off-by: Markus Armbruster <[e-mail]>
---
 docs/qapi-code-gen.txt   | 7 +++++++
 scripts/qapi-commands.py | 7 +++++++
 2 files changed, 14 insertions(+)

Patch

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);
     }
 
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())