[Qemu-devel] [PATCH v3 10/15] qapi: check invalid arguments on no-args commands

[marcandre.lureau@redhat.com]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The generated marshal functions do not visit arguments from commands
that take no arguments. Thus they fail to catch invalid
members. Visit the arguments, if provided, to throw an error in case of
invalid members.

static void qmp_marshal_stop(QDict *args, QObject **ret, Error **errp)
 {
+    Visitor *v = NULL;
     Error *err = NULL;
-
-    (void)args;
+    if (args) {
+        v = qmp_input_visitor_new(QOBJECT(args), true);
+        visit_start_struct(v, NULL, NULL, 0, &err);
+        if (err) {
+            goto out;
+        }
+
+        if (!err) {
+            visit_check_struct(v, &err);
+        }
+        visit_end_struct(v, NULL);
+        if (err) {
+            goto out;
+        }
+    }

     qmp_stop(&err);
+
+out:
     error_propagate(errp, err);
+    visit_free(v);
+    if (args) {
+        v = qapi_dealloc_visitor_new();
+        visit_start_struct(v, NULL, NULL, 0, NULL);
+
+        visit_end_struct(v, NULL);
+        visit_free(v);
+    }
 }

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/test-qmp-commands.c | 15 ++++++++++++++
 scripts/qapi-commands.py  | 51 ++++++++++++++++++++++++++++++-----------------
 2 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 261fd9e..81cbe54 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -106,6 +106,7 @@ static void test_dispatch_cmd(void)
 static void test_dispatch_cmd_failure(void)
 {
     QDict *req = qdict_new();
+    QDict *args = qdict_new();
     QObject *resp;
 
     qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd2")));
@@ -114,6 +115,20 @@ static void test_dispatch_cmd_failure(void)
     assert(resp != NULL);
     assert(qdict_haskey(qobject_to_qdict(resp), "error"));
 
+    qobject_decref(resp);
+    QDECREF(req);
+
+    /* check that with extra arguments it throws an error */
+    req = qdict_new();
+    qdict_put(args, "a", qint_from_int(66));
+    qdict_put(req, "arguments", args);
+
+    qdict_put_obj(req, "execute", QOBJECT(qstring_from_str("user_def_cmd")));
+
+    resp = qmp_dispatch(QOBJECT(req));
+    assert(resp != NULL);
+    assert(qdict_haskey(qobject_to_qdict(resp), "error"));
+
     qobject_decref(resp);
     QDECREF(req);
 }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 11aa54b..735e463 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -97,11 +97,27 @@ def gen_marshal_decl(name, export):
                  proto=gen_marshal_proto(name, export))
 
 
+def if_args(arg_type, block):
+    ret = ''
+    if not arg_type or arg_type.is_empty():
+        ret += mcgen('''
+    if (args) {
+''')
+        push_indent()
+    ret += block()
+    if not arg_type or arg_type.is_empty():
+        pop_indent()
+        ret += mcgen('''
+    }
+''')
+    return ret
+
 def gen_marshal(name, arg_type, boxed, ret_type, export):
     ret = mcgen('''
 
 %(proto)s
 {
+    Visitor *v = NULL;
     Error *err = NULL;
 ''',
                 proto=gen_marshal_proto(name, export))
@@ -112,17 +128,23 @@ def gen_marshal(name, arg_type, boxed, ret_type, export):
 ''',
                      c_type=ret_type.c_type())
 
+    visit_members = ''
     if arg_type and not arg_type.is_empty():
+        visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg, &err);',
+                            c_name=arg_type.c_name())
         ret += mcgen('''
-    Visitor *v;
     %(c_name)s arg = {0};
 
+''',
+                     c_name=arg_type.c_name())
+
+    ret += if_args(arg_type, lambda: mcgen('''
     v = qmp_input_visitor_new(QOBJECT(args), true);
     visit_start_struct(v, NULL, NULL, 0, &err);
     if (err) {
         goto out;
     }
-    visit_type_%(c_name)s_members(v, &arg, &err);
+    %(visit_members)s
     if (!err) {
         visit_check_struct(v, &err);
     }
@@ -131,35 +153,28 @@ def gen_marshal(name, arg_type, boxed, ret_type, export):
         goto out;
     }
 ''',
-                     c_name=arg_type.c_name())
-
-    else:
-        ret += mcgen('''
-
-    (void)args;
-''')
+                 visit_members=visit_members))
 
     ret += gen_call(name, arg_type, boxed, ret_type)
 
     # 'goto out' produced above for arg_type, and by gen_call() for ret_type
-    if (arg_type and not arg_type.is_empty()) or ret_type:
-        ret += mcgen('''
+    if arg_type and not arg_type.is_empty():
+        visit_members = mcgen('visit_type_%(c_name)s_members(v, &arg, NULL);',
+                              c_name=arg_type.c_name())
+    ret += mcgen('''
 
 out:
-''')
-    ret += mcgen('''
     error_propagate(errp, err);
-''')
-    if arg_type and not arg_type.is_empty():
-        ret += mcgen('''
     visit_free(v);
+''')
+    ret += if_args(arg_type, lambda: mcgen('''
     v = qapi_dealloc_visitor_new();
     visit_start_struct(v, NULL, NULL, 0, NULL);
-    visit_type_%(c_name)s_members(v, &arg, NULL);
+    %(visit_members)s
     visit_end_struct(v, NULL);
     visit_free(v);
 ''',
-                     c_name=arg_type.c_name())
+                     visit_members=visit_members))
 
     ret += mcgen('''
 }
-- 
2.9.0