[Qemu-devel] [PULL 0/2] QMP queue

[Qemu-devel] [PULL 0/2] QMP queue

[Luiz Capitulino]

The following changes since commit 6453a3a69488196f26d12654c6b148446abdf3d6:

  Merge remote-tracking branch 'quintela/migration.next' into staging (2013-07-15 14:49:16 -0500)

are available in the git repository at:


  git://repo.or.cz/qemu/qmp-unstable.git queue/qmp

for you to fetch changes up to f9b1d9b20f5d25b95f67a498e312f625d168fc51:

  qmp: update send-key document (2013-07-17 16:00:26 -0400)

----------------------------------------------------------------
Amos Kong (1):
      qmp: update send-key document

Luiz Capitulino (1):
      qapi: qapi-commands: fix possible leaks on visitor dealloc

 qmp-commands.hx          |  7 +++++--
 scripts/qapi-commands.py | 17 ++++++++++-------
 2 files changed, 15 insertions(+), 9 deletions(-)

[Qemu-devel] [PULL 1/2] qapi: qapi-commands: fix possible leaks on visitor dealloc

[Luiz Capitulino]

In qmp-marshal.c the dealloc visitor calls use the same errp
pointer of the input visitor calls. This means that if any of
the input visitor calls fails, then the dealloc visitor will
return early, before freeing the object's memory.

Here's an example, consider this code:

int qmp_marshal_input_block_passwd(Monitor *mon, const QDict *qdict, QObject **ret)
{
	[...]

    char * device = NULL;
    char * password = NULL;

    mi = qmp_input_visitor_new_strict(QOBJECT(args));
    v = qmp_input_get_visitor(mi);
    visit_type_str(v, &device, "device", errp);
    visit_type_str(v, &password, "password", errp);
    qmp_input_visitor_cleanup(mi);

    if (error_is_set(errp)) {
        goto out;
    }
    qmp_block_passwd(device, password, errp);

out:
    md = qapi_dealloc_visitor_new();
    v = qapi_dealloc_get_visitor(md);
    visit_type_str(v, &device, "device", errp);
    visit_type_str(v, &password, "password", errp);
    qapi_dealloc_visitor_cleanup(md);

	[...]

    return 0;
}

Consider errp != NULL when the out label is reached, we're going
to leak device and password.

This patch fixes this by always passing errp=NULL for dealloc
visitors, meaning that we always try to free them regardless of
any previous failure. The above example would then be:

out:
    md = qapi_dealloc_visitor_new();
    v = qapi_dealloc_get_visitor(md);
    visit_type_str(v, &device, "device", NULL);
    visit_type_str(v, &password, "password", NULL);
    qapi_dealloc_visitor_cleanup(md);

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qapi-commands.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index e06332b..b12b696 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -128,12 +128,15 @@ bool has_%(argname)s = false;
 
 def gen_visitor_input_block(args, obj, dealloc=False):
     ret = ""
+    errparg = 'errp'
+
     if len(args) == 0:
         return ret
 
     push_indent()
 
     if dealloc:
+        errparg = 'NULL'
         ret += mcgen('''
 md = qapi_dealloc_visitor_new();
 v = qapi_dealloc_get_visitor(md);
@@ -148,22 +151,22 @@ v = qmp_input_get_visitor(mi);
     for argname, argtype, optional, structured in parse_args(args):
         if optional:
             ret += mcgen('''
-visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
+visit_start_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
 if (has_%(c_name)s) {
 ''',
-                         c_name=c_var(argname), name=argname)
+                         c_name=c_var(argname), name=argname, errp=errparg)
             push_indent()
         ret += mcgen('''
-%(visitor)s(v, &%(c_name)s, "%(name)s", errp);
+%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
                      c_name=c_var(argname), name=argname, argtype=argtype,
-                     visitor=type_visitor(argtype))
+                     visitor=type_visitor(argtype), errp=errparg)
         if optional:
             pop_indent()
             ret += mcgen('''
 }
-visit_end_optional(v, errp);
-''')
+visit_end_optional(v, %(errp)s);
+''', errp=errparg)
 
     if dealloc:
         ret += mcgen('''
@@ -194,7 +197,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_o
     }
     qmp_output_visitor_cleanup(mo);
     v = qapi_dealloc_get_visitor(md);
-    %(visitor)s(v, &ret_in, "unused", errp);
+    %(visitor)s(v, &ret_in, "unused", NULL);
     qapi_dealloc_visitor_cleanup(md);
 }
 ''',
-- 
1.8.1.4

[Qemu-devel] [PULL 2/2] qmp: update send-key document

[Luiz Capitulino]

From: Amos Kong <akong@redhat.com>

commit 9f328977 changes qmp_send_key() to accept key codes in hex,
but the document wasn't updated. The items of keys list is union
now, not enum.

Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qmp-commands.hx | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index e075df4..b8e7758 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -346,7 +346,8 @@ Send keys to VM.
 Arguments:
 
 keys array:
-    - "key": key sequence (a json-array of key enum values)
+    - "key": key sequence (a json-array of key union values,
+             union can be number or qcode enum)
 
 - hold-time: time to delay key up events, milliseconds. Defaults to 100
              (json-int, optional)
@@ -354,7 +355,9 @@ keys array:
 Example:
 
 -> { "execute": "send-key",
-     "arguments": { 'keys': [ 'ctrl', 'alt', 'delete' ] } }
+     "arguments": { "keys": [ { "type": "qcode", "data": "ctrl" },
+                              { "type": "qcode", "data": "alt" },
+                              { "type": "qcode", "data": "delete" } ] } }
 <- { "return": {} }
 
 EQMP
-- 
1.8.1.4