[Qemu-devel] [PATCH v3 00/14]: QMP: Replace client argument checker

[Qemu-devel] [PATCH v3 00/14]: QMP: Replace client argument checker

[Luiz Capitulino]

Current QMP's client argument checker code is more complex than it should be
and has a flaw: it ignores unknown arguments.

This series solves both problems by introducing a new, simple and ultra-poweful
argument checker. This wasn't trivial to get right due to the number of errors
combinations, so review is very appreciated.

changelog
---------

v2 -> v3

- Move all input object checking into qmp_check_input_obj()
- Fix remaining problem with the handling of O-type arguments
- Small renames suggested by Markus
- Small cleanup in handle_qmp_command()

v1 -> v2

- Introduce new iteration API and use it
- Handle O-type correctly (I hope so)
- Address several small issues found by Markus

[Qemu-devel] [PATCH 01/14] QDict: Rename 'err_value'

[Luiz Capitulino]

A missing key is not an error.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qdict.c |    6 +++---
 qdict.h |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/qdict.c b/qdict.c
index 175bc17..c974d6f 100644
--- a/qdict.c
+++ b/qdict.c
@@ -272,16 +272,16 @@ const char *qdict_get_str(const QDict *qdict, const char *key)
  *
  * Return integer mapped by 'key', if it is not present in
  * the dictionary or if the stored object is not of QInt type
- * 'err_value' will be returned.
+ * 'def_value' will be returned.
  */
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
-                          int64_t err_value)
+                          int64_t def_value)
 {
     QObject *obj;
 
     obj = qdict_get(qdict, key);
     if (!obj || qobject_type(obj) != QTYPE_QINT)
-        return err_value;
+        return def_value;
 
     return qint_get_int(qobject_to_qint(obj));
 }
diff --git a/qdict.h b/qdict.h
index 5e5902c..72ea563 100644
--- a/qdict.h
+++ b/qdict.h
@@ -56,7 +56,7 @@ QList *qdict_get_qlist(const QDict *qdict, const char *key);
 QDict *qdict_get_qdict(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
-                          int64_t err_value);
+                          int64_t def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 #endif /* QDICT_H */
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 02/14] QDict: Small terminology change

[Luiz Capitulino]

Let's call a 'hash' only what is returned by our hash function,
anything else is a 'bucket'.

This helps avoiding confusion with regard to how we traverse
our table.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 check-qdict.c |    2 +-
 qdict.c       |   24 ++++++++++++------------
 qdict.h       |    4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/check-qdict.c b/check-qdict.c
index 2c3089f..1b070f4 100644
--- a/check-qdict.c
+++ b/check-qdict.c
@@ -50,7 +50,7 @@ START_TEST(qdict_put_obj_test)
     qdict_put_obj(qdict, "", QOBJECT(qint_from_int(num)));
 
     fail_unless(qdict_size(qdict) == 1);
-    ent = QLIST_FIRST(&qdict->table[12345 % QDICT_HASH_SIZE]);
+    ent = QLIST_FIRST(&qdict->table[12345 % QDICT_BUCKET_MAX]);
     qi = qobject_to_qint(ent->value);
     fail_unless(qint_get_int(qi) == num);
 
diff --git a/qdict.c b/qdict.c
index c974d6f..71be2eb 100644
--- a/qdict.c
+++ b/qdict.c
@@ -86,11 +86,11 @@ static QDictEntry *alloc_entry(const char *key, QObject *value)
  * qdict_find(): List lookup function
  */
 static QDictEntry *qdict_find(const QDict *qdict,
-                              const char *key, unsigned int hash)
+                              const char *key, unsigned int bucket)
 {
     QDictEntry *entry;
 
-    QLIST_FOREACH(entry, &qdict->table[hash], next)
+    QLIST_FOREACH(entry, &qdict->table[bucket], next)
         if (!strcmp(entry->key, key))
             return entry;
 
@@ -110,11 +110,11 @@ static QDictEntry *qdict_find(const QDict *qdict,
  */
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value)
 {
-    unsigned int hash;
+    unsigned int bucket;
     QDictEntry *entry;
 
-    hash = tdb_hash(key) % QDICT_HASH_SIZE;
-    entry = qdict_find(qdict, key, hash);
+    bucket = tdb_hash(key) % QDICT_BUCKET_MAX;
+    entry = qdict_find(qdict, key, bucket);
     if (entry) {
         /* replace key's value */
         qobject_decref(entry->value);
@@ -122,7 +122,7 @@ void qdict_put_obj(QDict *qdict, const char *key, QObject *value)
     } else {
         /* allocate a new entry */
         entry = alloc_entry(key, value);
-        QLIST_INSERT_HEAD(&qdict->table[hash], entry, next);
+        QLIST_INSERT_HEAD(&qdict->table[bucket], entry, next);
         qdict->size++;
     }
 }
@@ -137,7 +137,7 @@ QObject *qdict_get(const QDict *qdict, const char *key)
 {
     QDictEntry *entry;
 
-    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
     return (entry == NULL ? NULL : entry->value);
 }
 
@@ -148,8 +148,8 @@ QObject *qdict_get(const QDict *qdict, const char *key)
  */
 int qdict_haskey(const QDict *qdict, const char *key)
 {
-    unsigned int hash = tdb_hash(key) % QDICT_HASH_SIZE;
-    return (qdict_find(qdict, key, hash) == NULL ? 0 : 1);
+    unsigned int bucket = tdb_hash(key) % QDICT_BUCKET_MAX;
+    return (qdict_find(qdict, key, bucket) == NULL ? 0 : 1);
 }
 
 /**
@@ -318,7 +318,7 @@ void qdict_iter(const QDict *qdict,
     int i;
     QDictEntry *entry;
 
-    for (i = 0; i < QDICT_HASH_SIZE; i++) {
+    for (i = 0; i < QDICT_BUCKET_MAX; i++) {
         QLIST_FOREACH(entry, &qdict->table[i], next)
             iter(entry->key, entry->value, opaque);
     }
@@ -347,7 +347,7 @@ void qdict_del(QDict *qdict, const char *key)
 {
     QDictEntry *entry;
 
-    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+    entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_BUCKET_MAX);
     if (entry) {
         QLIST_REMOVE(entry, next);
         qentry_destroy(entry);
@@ -366,7 +366,7 @@ static void qdict_destroy_obj(QObject *obj)
     assert(obj != NULL);
     qdict = qobject_to_qdict(obj);
 
-    for (i = 0; i < QDICT_HASH_SIZE; i++) {
+    for (i = 0; i < QDICT_BUCKET_MAX; i++) {
         QDictEntry *entry = QLIST_FIRST(&qdict->table[i]);
         while (entry) {
             QDictEntry *tmp = QLIST_NEXT(entry, next);
diff --git a/qdict.h b/qdict.h
index 72ea563..dcd2b29 100644
--- a/qdict.h
+++ b/qdict.h
@@ -18,7 +18,7 @@
 #include "qemu-queue.h"
 #include <stdint.h>
 
-#define QDICT_HASH_SIZE 512
+#define QDICT_BUCKET_MAX 512
 
 typedef struct QDictEntry {
     char *key;
@@ -29,7 +29,7 @@ typedef struct QDictEntry {
 typedef struct QDict {
     QObject_HEAD;
     size_t size;
-    QLIST_HEAD(,QDictEntry) table[QDICT_HASH_SIZE];
+    QLIST_HEAD(,QDictEntry) table[QDICT_BUCKET_MAX];
 } QDict;
 
 /* Object API */
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 03/14] QDict: Introduce functions to retrieve QDictEntry values

[Luiz Capitulino]

Next commit will introduce a new QDict iteration API which
returns QDictEntry entries, but we don't want users to directly
access its members since QDictEntry should be private to QDict.

In the near future this kind of data type will be turned into a
forward reference.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qdict.c |   21 +++++++++++++++++++++
 qdict.h |    2 ++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index 71be2eb..c467763 100644
--- a/qdict.c
+++ b/qdict.c
@@ -83,6 +83,27 @@ static QDictEntry *alloc_entry(const char *key, QObject *value)
 }
 
 /**
+ * qdict_entry_value(): Return qdict entry value
+ *
+ * Return weak reference.
+ */
+QObject *qdict_entry_value(const QDictEntry *entry)
+{
+    return entry->value;
+}
+
+/**
+ * qdict_entry_key(): Return qdict entry key
+ *
+ * Return a *pointer* to the string, it has to be duplicated before being
+ * stored.
+ */
+const char *qdict_entry_key(const QDictEntry *entry)
+{
+    return entry->key;
+}
+
+/**
  * qdict_find(): List lookup function
  */
 static QDictEntry *qdict_find(const QDict *qdict,
diff --git a/qdict.h b/qdict.h
index dcd2b29..0c8de3c 100644
--- a/qdict.h
+++ b/qdict.h
@@ -34,6 +34,8 @@ typedef struct QDict {
 
 /* Object API */
 QDict *qdict_new(void);
+const char *qdict_entry_key(const QDictEntry *entry);
+QObject *qdict_entry_value(const QDictEntry *entry);
 size_t qdict_size(const QDict *qdict);
 void qdict_put_obj(QDict *qdict, const char *key, QObject *value);
 void qdict_del(QDict *qdict, const char *key);
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 04/14] QDict: Introduce new iteration API

[Luiz Capitulino]

It's composed of functions qdict_first() and qdict_next(), plus
functions to access QDictEntry values.

This API was suggested by Markus Armbruster <armbru@redhat.com> and
it offers full control over the iteration process.

The usage is simple, the following example prints all keys in 'qdict'
(it's hopefully better than any English description):

   QDict *qdict;
   const QDictEntry *ent;

   [...]

   for (ent = qdict_first(qdict); ent; ent = qdict_next(qdict, ent)) {
        printf("%s ", qdict_entry_key(ent));
    }

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qdict.c |   37 +++++++++++++++++++++++++++++++++++++
 qdict.h |    2 ++
 2 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index c467763..a28a0a9 100644
--- a/qdict.c
+++ b/qdict.c
@@ -345,6 +345,43 @@ void qdict_iter(const QDict *qdict,
     }
 }
 
+static QDictEntry *qdict_next_entry(const QDict *qdict, int first_bucket)
+{
+    int i;
+
+    for (i = first_bucket; i < QDICT_BUCKET_MAX; i++) {
+        if (!QLIST_EMPTY(&qdict->table[i])) {
+            return QLIST_FIRST(&qdict->table[i]);
+        }
+    }
+
+    return NULL;
+}
+
+/**
+ * qdict_first(): Return first qdict entry for iteration.
+ */
+const QDictEntry *qdict_first(const QDict *qdict)
+{
+    return qdict_next_entry(qdict, 0);
+}
+
+/**
+ * qdict_next(): Return next qdict entry in an iteration.
+ */
+const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry)
+{
+    QDictEntry *ret;
+
+    ret = QLIST_NEXT(entry, next);
+    if (!ret) {
+        unsigned int bucket = tdb_hash(entry->key) % QDICT_BUCKET_MAX;
+        ret = qdict_next_entry(qdict, bucket + 1);
+    }
+
+    return ret;
+}
+
 /**
  * qentry_destroy(): Free all the memory allocated by a QDictEntry
  */
diff --git a/qdict.h b/qdict.h
index 0c8de3c..0e7a43f 100644
--- a/qdict.h
+++ b/qdict.h
@@ -45,6 +45,8 @@ QDict *qobject_to_qdict(const QObject *obj);
 void qdict_iter(const QDict *qdict,
                 void (*iter)(const char *key, QObject *obj, void *opaque),
                 void *opaque);
+const QDictEntry *qdict_first(const QDict *qdict);
+const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
 
 /* Helper to qdict_put_obj(), accepts any object */
 #define qdict_put(qdict, key, obj) \
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 05/14] check-qdict: Introduce test for the new iteration API

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 check-qdict.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/check-qdict.c b/check-qdict.c
index 1b070f4..6afce5a 100644
--- a/check-qdict.c
+++ b/check-qdict.c
@@ -194,6 +194,36 @@ START_TEST(qobject_to_qdict_test)
 }
 END_TEST
 
+START_TEST(qdict_iterapi_test)
+{
+    int count;
+    const QDictEntry *ent;
+
+    fail_unless(qdict_first(tests_dict) == NULL);
+
+    qdict_put(tests_dict, "key1", qint_from_int(1));
+    qdict_put(tests_dict, "key2", qint_from_int(2));
+    qdict_put(tests_dict, "key3", qint_from_int(3));
+
+    count = 0;
+    for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, ent)){
+        fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1);
+        count++;
+    }
+
+    fail_unless(count == qdict_size(tests_dict));
+
+    /* Do it again to test restarting */
+    count = 0;
+    for (ent = qdict_first(tests_dict); ent; ent = qdict_next(tests_dict, ent)){
+        fail_unless(qdict_haskey(tests_dict, qdict_entry_key(ent)) == 1);
+        count++;
+    }
+
+    fail_unless(count == qdict_size(tests_dict));
+}
+END_TEST
+
 /*
  * Errors test-cases
  */
@@ -338,6 +368,7 @@ static Suite *qdict_suite(void)
     tcase_add_test(qdict_public2_tcase, qdict_haskey_test);
     tcase_add_test(qdict_public2_tcase, qdict_del_test);
     tcase_add_test(qdict_public2_tcase, qobject_to_qdict_test);
+    tcase_add_test(qdict_public2_tcase, qdict_iterapi_test);
 
     qdict_errors_tcase = tcase_create("Errors");
     suite_add_tcase(s, qdict_errors_tcase);
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 06/14] QDict: Introduce qdict_get_try_bool()

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qdict.c |   18 ++++++++++++++++++
 qdict.h |    1 +
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/qdict.c b/qdict.c
index a28a0a9..dee0fb4 100644
--- a/qdict.c
+++ b/qdict.c
@@ -308,6 +308,24 @@ int64_t qdict_get_try_int(const QDict *qdict, const char *key,
 }
 
 /**
+ * qdict_get_try_bool(): Try to get a bool mapped by 'key'
+ *
+ * Return bool mapped by 'key', if it is not present in the
+ * dictionary or if the stored object is not of QBool type
+ * 'def_value' will be returned.
+ */
+int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value)
+{
+    QObject *obj;
+
+    obj = qdict_get(qdict, key);
+    if (!obj || qobject_type(obj) != QTYPE_QBOOL)
+        return def_value;
+
+    return qbool_get_int(qobject_to_qbool(obj));
+}
+
+/**
  * qdict_get_try_str(): Try to get a pointer to the stored string
  * mapped by 'key'
  *
diff --git a/qdict.h b/qdict.h
index 0e7a43f..929d8d2 100644
--- a/qdict.h
+++ b/qdict.h
@@ -61,6 +61,7 @@ QDict *qdict_get_qdict(const QDict *qdict, const char *key);
 const char *qdict_get_str(const QDict *qdict, const char *key);
 int64_t qdict_get_try_int(const QDict *qdict, const char *key,
                           int64_t def_value);
+int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value);
 const char *qdict_get_try_str(const QDict *qdict, const char *key);
 
 #endif /* QDICT_H */
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 07/14] Monitor: handle optional '-' arg as a bool

[Luiz Capitulino]

Historically, user monitor arguments beginning with '-' (eg. '-f')
were passed as integers down to handlers.

I've maintained this behavior in the new monitor because we didn't
have a boolean type at the very beginning of QMP. Today we have it
and this behavior is causing trouble to QMP's argument checker.

This commit fixes the problem by doing the following changes:

1. User Monitor

   Before: the optional arg was represented as a QInt, we'd pass 1
           down to handlers if the user specified the argument or
           0 otherwise

   This commit: the optional arg is represented as a QBool, we pass
                true down to handlers if the user specified the
                argument, otherwise _nothing_ is passed

2. QMP

   Before: the client was required to pass the arg as QBool, but we'd
           convert it to QInt internally. If the argument wasn't passed,
           we'd pass 0 down

   This commit: still require a QBool, but doesn't do any conversion and
                doesn't pass any default value

3. Convert existing handlers (do_eject()/do_migrate()) to the new way

   Before: Both handlers would expect a QInt value, either 0 or 1

   This commit: Change the handlers to accept a QBool, they handle the
                following cases:

                   A) true is passed: the option is enabled
                   B) false is passed: the option is disabled
                   C) nothing is passed: option not specified, use
                                         default behavior

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 blockdev.c  |    2 +-
 migration.c |   16 +++++++---------
 monitor.c   |   17 +++--------------
 3 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 3b8c606..4dcfad8 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -521,7 +521,7 @@ static int eject_device(Monitor *mon, BlockDriverState *bs, int force)
 int do_eject(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     BlockDriverState *bs;
-    int force = qdict_get_int(qdict, "force");
+    int force = qdict_get_try_bool(qdict, "force", 0);
     const char *filename = qdict_get_str(qdict, "device");
 
     bs = bdrv_find(filename);
diff --git a/migration.c b/migration.c
index b49964c..650eb78 100644
--- a/migration.c
+++ b/migration.c
@@ -75,7 +75,9 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = NULL;
     const char *p;
-    int detach = qdict_get_int(qdict, "detach");
+    int detach = qdict_get_try_bool(qdict, "detach", 0);
+    int blk = qdict_get_try_bool(qdict, "blk", 0);
+    int inc = qdict_get_try_bool(qdict, "inc", 0);
     const char *uri = qdict_get_str(qdict, "uri");
 
     if (current_migration &&
@@ -86,21 +88,17 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 
     if (strstart(uri, "tcp:", &p)) {
         s = tcp_start_outgoing_migration(mon, p, max_throttle, detach,
-                                         (int)qdict_get_int(qdict, "blk"), 
-                                         (int)qdict_get_int(qdict, "inc"));
+                                         blk, inc);
 #if !defined(WIN32)
     } else if (strstart(uri, "exec:", &p)) {
         s = exec_start_outgoing_migration(mon, p, max_throttle, detach,
-                                          (int)qdict_get_int(qdict, "blk"), 
-                                          (int)qdict_get_int(qdict, "inc"));
+                                          blk, inc);
     } else if (strstart(uri, "unix:", &p)) {
         s = unix_start_outgoing_migration(mon, p, max_throttle, detach,
-					  (int)qdict_get_int(qdict, "blk"), 
-                                          (int)qdict_get_int(qdict, "inc"));
+                                          blk, inc);
     } else if (strstart(uri, "fd:", &p)) {
         s = fd_start_outgoing_migration(mon, p, max_throttle, detach, 
-                                        (int)qdict_get_int(qdict, "blk"), 
-                                        (int)qdict_get_int(qdict, "inc"));
+                                        blk, inc);
 #endif
     } else {
         monitor_printf(mon, "unknown migration protocol: %s\n", uri);
diff --git a/monitor.c b/monitor.c
index ba7d5d9..5084a6e 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3565,7 +3565,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
         case '-':
             {
                 const char *tmp = p;
-                int has_option, skip_key = 0;
+                int skip_key = 0;
                 /* option */
 
                 c = *typestr++;
@@ -3573,7 +3573,6 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     goto bad_type;
                 while (qemu_isspace(*p))
                     p++;
-                has_option = 0;
                 if (*p == '-') {
                     p++;
                     if(c != *p) {
@@ -3589,11 +3588,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     if(skip_key) {
                         p = tmp;
                     } else {
+                        /* has option */
                         p++;
-                        has_option = 1;
+                        qdict_put(qdict, key, qbool_from_int(1));
                     }
                 }
-                qdict_put(qdict, key, qint_from_int(has_option));
             }
             break;
         default:
@@ -3978,11 +3977,6 @@ static int check_opt(const CmdArgs *cmd_args, const char *name, QDict *args)
         return -1;
     }
 
-    if (cmd_args->type == '-') {
-        /* handlers expect a value, they need to be changed */
-        qdict_put(args, name, qint_from_int(0));
-    }
-
     return 0;
 }
 
@@ -4055,11 +4049,6 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
                 qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
                 return -1;
             }
-            if (qobject_type(value) == QTYPE_QBOOL) {
-                /* handlers expect a QInt, they need to be changed */
-                qdict_put(args, name,
-                         qint_from_int(qbool_get_int(qobject_to_qbool(value))));
-            }
             break;
         case 'O':
         default:
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 08/14] QMP: New argument checker (first part)

[Luiz Capitulino]

Current QMP's argument checker is more complex than it should be
and has (at least) one serious bug: it ignores unknown arguments.

To solve both problems we introduce a new argument checker. It's
added on top of the existing one, so that there are no regressions
during the transition.

This commit introduces the first part of the new checker, which
is run by qmp_check_client_args() and does the following:

  1. Check if all mandatory arguments were provided
  2. Set flags for argument validation

In order to do that, we transform the args_type string (from
qemu-montor.hx) into a qdict and iterate over it.

Next commit adds the new checker's second part: type checking and
invalid argument detection.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |  106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 5084a6e..edc362f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -177,6 +177,9 @@ static inline void mon_print_count_init(Monitor *mon) { }
 static inline int mon_print_count_get(const Monitor *mon) { return 0; }
 #endif /* CONFIG_DEBUG_MONITOR */
 
+/* QMP checker flags */
+#define QMP_ACCEPT_UNKNOWNS 1
+
 static QLIST_HEAD(mon_list, Monitor) mon_list;
 
 static const mon_cmd_t mon_cmds[];
@@ -4140,6 +4143,104 @@ static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
     return (qmp_cmd_mode(mon) ? is_cap : !is_cap);
 }
 
+/*
+ * - Check if the client has passed all mandatory args
+ * - Set special flags for argument validation
+ */
+static int check_mandatory_args(const QDict *cmd_args,
+                                const QDict *client_args, int *flags)
+{
+    const QDictEntry *ent;
+
+    for (ent = qdict_first(cmd_args); ent; ent = qdict_next(cmd_args, ent)) {
+        const char *cmd_arg_name = qdict_entry_key(ent);
+        QString *type = qobject_to_qstring(qdict_entry_value(ent));
+        assert(type != NULL);
+
+        if (qstring_get_str(type)[0] == 'O') {
+            assert((*flags & QMP_ACCEPT_UNKNOWNS) == 0);
+            *flags |= QMP_ACCEPT_UNKNOWNS;
+        } else if (qstring_get_str(type)[0] != '-' &&
+                   qstring_get_str(type)[1] != '?' &&
+                   !qdict_haskey(client_args, cmd_arg_name)) {
+            qerror_report(QERR_MISSING_PARAMETER, cmd_arg_name);
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static QDict *qdict_from_args_type(const char *args_type)
+{
+    int i;
+    QDict *qdict;
+    QString *key, *type, *cur_qs;
+
+    assert(args_type != NULL);
+
+    qdict = qdict_new();
+
+    if (args_type == NULL || args_type[0] == '\0') {
+        /* no args, empty qdict */
+        goto out;
+    }
+
+    key = qstring_new();
+    type = qstring_new();
+
+    cur_qs = key;
+
+    for (i = 0;; i++) {
+        switch (args_type[i]) {
+            case ',':
+            case '\0':
+                qdict_put(qdict, qstring_get_str(key), type);
+                QDECREF(key);
+                if (args_type[i] == '\0') {
+                    goto out;
+                }
+                type = qstring_new(); /* qdict has ref */
+                cur_qs = key = qstring_new();
+                break;
+            case ':':
+                cur_qs = type;
+                break;
+            default:
+                qstring_append_chr(cur_qs, args_type[i]);
+                break;
+        }
+    }
+
+out:
+    return qdict;
+}
+
+/*
+ * Client argument checking rules:
+ *
+ * 1. Client must provide all mandatory arguments
+ */
+static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
+{
+    int flags, err;
+    QDict *cmd_args;
+
+    cmd_args = qdict_from_args_type(cmd->args_type);
+
+    flags = 0;
+    err = check_mandatory_args(cmd_args, client_args, &flags);
+    if (err) {
+        goto out;
+    }
+
+    /* TODO: Check client args type */
+
+out:
+    QDECREF(cmd_args);
+    return err;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
     int err;
@@ -4215,6 +4316,11 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 
     QDECREF(input);
 
+    err = qmp_check_client_args(cmd, args);
+    if (err < 0) {
+        goto err_out;
+    }
+
     err = monitor_check_qmp_args(cmd, args);
     if (err < 0) {
         goto err_out;
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 09/14] QMP: New argument checker (second part)

[Luiz Capitulino]

This commit introduces the second (and last) part of QMP's new
argument checker.

The job is done by check_client_args_type(), it iterates over
the client's argument qdict and for for each argument it checks
if it exists and if its type is valid.

It's important to observe the following changes from the existing
argument checker:

  - If the handler accepts an O-type argument, unknown arguments
    are passed down to it. It's up to O-type handlers to validate
    their arguments

  - Boolean types (eg. 'b' and '-') don't accept integers anymore,
    only json-bool

  - Argument types '/' and '.' are currently unsupported under QMP,
    thus they're not handled

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index edc362f..602ccdd 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4144,6 +4144,95 @@ static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
 }
 
 /*
+ * Argument validation rules:
+ *
+ * 1. The argument must exist in cmd_args qdict
+ * 2. The argument type must be the expected one
+ *
+ * Special case: If the argument doesn't exist in cmd_args and
+ *               the QMP_ACCEPT_UNKNOWNS flag is set, then the
+ *               checking is skipped for it.
+ */
+static int check_client_args_type(const QDict *client_args,
+                                  const QDict *cmd_args, int flags)
+{
+    const QDictEntry *ent;
+
+    for (ent = qdict_first(client_args); ent;ent = qdict_next(client_args,ent)){
+        QObject *obj;
+        QString *arg_type;
+        const QObject *client_arg = qdict_entry_value(ent);
+        const char *client_arg_name = qdict_entry_key(ent);
+
+        obj = qdict_get(cmd_args, client_arg_name);
+        if (!obj) {
+            if (flags & QMP_ACCEPT_UNKNOWNS) {
+                /* handler accepts unknowns */
+                continue;
+            }
+            /* client arg doesn't exist */
+            qerror_report(QERR_INVALID_PARAMETER, client_arg_name);
+            return -1;
+        }
+
+        arg_type = qobject_to_qstring(obj);
+        assert(arg_type != NULL);
+
+        /* check if argument's type is correct */
+        switch (qstring_get_str(arg_type)[0]) {
+        case 'F':
+        case 'B':
+        case 's':
+            if (qobject_type(client_arg) != QTYPE_QSTRING) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "string");
+                return -1;
+            }
+        break;
+        case 'i':
+        case 'l':
+        case 'M':
+            if (qobject_type(client_arg) != QTYPE_QINT) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "int");
+                return -1; 
+            }
+            break;
+        case 'f':
+        case 'T':
+            if (qobject_type(client_arg) != QTYPE_QINT &&
+                qobject_type(client_arg) != QTYPE_QFLOAT) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "number");
+               return -1; 
+            }
+            break;
+        case 'b':
+        case '-':
+            if (qobject_type(client_arg) != QTYPE_QBOOL) {
+                qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
+                              "bool");
+               return -1; 
+            }
+            break;
+        case 'O':
+            assert(flags & QMP_ACCEPT_UNKNOWNS);
+            break;
+        case '/':
+        case '.':
+            /*
+             * These types are not supported by QMP and thus are not
+             * handled here. Fall through.
+             */
+        default:
+            abort();
+        }
+    }
+
+    return 0;
+}
+
+/*
  * - Check if the client has passed all mandatory args
  * - Set special flags for argument validation
  */
@@ -4220,6 +4309,9 @@ out:
  * Client argument checking rules:
  *
  * 1. Client must provide all mandatory arguments
+ * 2. Each argument provided by the client must be expected
+ * 3. Each argument provided by the client must have the type expected
+ *    by the command
  */
 static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
 {
@@ -4234,7 +4326,7 @@ static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
         goto out;
     }
 
-    /* TODO: Check client args type */
+    err = check_client_args_type(client_args, cmd_args, flags);
 
 out:
     QDECREF(cmd_args);
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 10/14] QMP: Drop old client argument checker

[Luiz Capitulino]

Previous two commits added qmp_check_client_args(), which
fully replaces this code and is way better.

It's important to note that the new checker doesn't support
the '/' arg type. As we don't have any of those handlers
converted to QMP, this is just dead code.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |  176 -------------------------------------------------------------
 1 files changed, 0 insertions(+), 176 deletions(-)

diff --git a/monitor.c b/monitor.c
index 602ccdd..8d3e788 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3966,177 +3966,6 @@ static int monitor_can_read(void *opaque)
     return (mon->suspend_cnt == 0) ? 1 : 0;
 }
 
-typedef struct CmdArgs {
-    QString *name;
-    int type;
-    int flag;
-    int optional;
-} CmdArgs;
-
-static int check_opt(const CmdArgs *cmd_args, const char *name, QDict *args)
-{
-    if (!cmd_args->optional) {
-        qerror_report(QERR_MISSING_PARAMETER, name);
-        return -1;
-    }
-
-    return 0;
-}
-
-static int check_arg(const CmdArgs *cmd_args, QDict *args)
-{
-    QObject *value;
-    const char *name;
-
-    name = qstring_get_str(cmd_args->name);
-
-    if (!args) {
-        return check_opt(cmd_args, name, args);
-    }
-
-    value = qdict_get(args, name);
-    if (!value) {
-        return check_opt(cmd_args, name, args);
-    }
-
-    switch (cmd_args->type) {
-        case 'F':
-        case 'B':
-        case 's':
-            if (qobject_type(value) != QTYPE_QSTRING) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "string");
-                return -1;
-            }
-            break;
-        case '/': {
-            int i;
-            const char *keys[] = { "count", "format", "size", NULL };
-
-            for (i = 0; keys[i]; i++) {
-                QObject *obj = qdict_get(args, keys[i]);
-                if (!obj) {
-                    qerror_report(QERR_MISSING_PARAMETER, name);
-                    return -1;
-                }
-                if (qobject_type(obj) != QTYPE_QINT) {
-                    qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
-                    return -1;
-                }
-            }
-            break;
-        }
-        case 'i':
-        case 'l':
-        case 'M':
-            if (qobject_type(value) != QTYPE_QINT) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "int");
-                return -1;
-            }
-            break;
-        case 'f':
-        case 'T':
-            if (qobject_type(value) != QTYPE_QINT && qobject_type(value) != QTYPE_QFLOAT) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "number");
-                return -1;
-            }
-            break;
-        case 'b':
-            if (qobject_type(value) != QTYPE_QBOOL) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
-                return -1;
-            }
-            break;
-        case '-':
-            if (qobject_type(value) != QTYPE_QINT &&
-                qobject_type(value) != QTYPE_QBOOL) {
-                qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "bool");
-                return -1;
-            }
-            break;
-        case 'O':
-        default:
-            /* impossible */
-            abort();
-    }
-
-    return 0;
-}
-
-static void cmd_args_init(CmdArgs *cmd_args)
-{
-    cmd_args->name = qstring_new();
-    cmd_args->type = cmd_args->flag = cmd_args->optional = 0;
-}
-
-static int check_opts(QemuOptsList *opts_list, QDict *args)
-{
-    assert(!opts_list->desc->name);
-    return 0;
-}
-
-/*
- * This is not trivial, we have to parse Monitor command's argument
- * type syntax to be able to check the arguments provided by clients.
- *
- * In the near future we will be using an array for that and will be
- * able to drop all this parsing...
- */
-static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args)
-{
-    int err;
-    const char *p;
-    CmdArgs cmd_args;
-    QemuOptsList *opts_list;
-
-    if (cmd->args_type == NULL) {
-        return (qdict_size(args) == 0 ? 0 : -1);
-    }
-
-    err = 0;
-    cmd_args_init(&cmd_args);
-    opts_list = NULL;
-
-    for (p = cmd->args_type;; p++) {
-        if (*p == ':') {
-            cmd_args.type = *++p;
-            p++;
-            if (cmd_args.type == '-') {
-                cmd_args.flag = *p++;
-                cmd_args.optional = 1;
-            } else if (cmd_args.type == 'O') {
-                opts_list = qemu_find_opts(qstring_get_str(cmd_args.name));
-                assert(opts_list);
-            } else if (*p == '?') {
-                cmd_args.optional = 1;
-                p++;
-            }
-
-            assert(*p == ',' || *p == '\0');
-            if (opts_list) {
-                err = check_opts(opts_list, args);
-                opts_list = NULL;
-            } else {
-                err = check_arg(&cmd_args, args);
-                QDECREF(cmd_args.name);
-                cmd_args_init(&cmd_args);
-            }
-
-            if (err < 0) {
-                break;
-            }
-        } else {
-            qstring_append_chr(cmd_args.name, *p);
-        }
-
-        if (*p == '\0') {
-            break;
-        }
-    }
-
-    QDECREF(cmd_args.name);
-    return err;
-}
-
 static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name)
 {
     int is_cap = compare_cmd(cmd_name, "qmp_capabilities");
@@ -4413,11 +4242,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         goto err_out;
     }
 
-    err = monitor_check_qmp_args(cmd, args);
-    if (err < 0) {
-        goto err_out;
-    }
-
     if (monitor_handler_is_async(cmd)) {
         err = qmp_async_cmd_handler(mon, cmd, args);
         if (err) {
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 11/14] QError: Introduce QERR_QMP_EXTRA_MEMBER

[Luiz Capitulino]

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qerror.c |    4 ++++
 qerror.h |    3 +++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/qerror.c b/qerror.c
index cce1e7b..2f6f590 100644
--- a/qerror.c
+++ b/qerror.c
@@ -177,6 +177,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "QMP input object member '%(member)' expects '%(expected)'",
     },
     {
+        .error_fmt = QERR_QMP_EXTRA_MEMBER,
+        .desc      = "QMP input object member '%(member)' is unexpected",
+    },
+    {
         .error_fmt = QERR_SET_PASSWD_FAILED,
         .desc      = "Could not set password",
     },
diff --git a/qerror.h b/qerror.h
index 77ae574..9ad00b4 100644
--- a/qerror.h
+++ b/qerror.h
@@ -148,6 +148,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_QMP_BAD_INPUT_OBJECT_MEMBER \
     "{ 'class': 'QMPBadInputObjectMember', 'data': { 'member': %s, 'expected': %s } }"
 
+#define QERR_QMP_EXTRA_MEMBER \
+    "{ 'class': 'QMPExtraInputObjectMember', 'data': { 'member': %s } }"
+
 #define QERR_SET_PASSWD_FAILED \
     "{ 'class': 'SetPasswdFailed', 'data': {} }"
 
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 12/14] QMP: Introduce qmp_check_input_obj()

[Luiz Capitulino]

This is similar to qmp_check_client_args(), but it checks if
the input object follows the specification (QMP/qmp-spec.txt
section 2.3).

As we're limited to three keys, the work here is quite simple:
we iterate over the input object, checking each time if the
current argument complies to the specification.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 61 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index 8d3e788..fab553a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4162,6 +4162,62 @@ out:
     return err;
 }
 
+/*
+ * Input object checking rules
+ *
+ * 1. Input object must be a dict
+ * 2. The "execute" key must exist
+ * 3. The "execute" key must be a string
+ * 4. If the "arguments" key exists, it must be a dict
+ * 5. If the "id" key exists, it can be anything (ie. json-value)
+ * 6. Any argument not listed above is considered invalid
+ */
+static QDict *qmp_check_input_obj(QObject *input_obj)
+{
+    const QDictEntry *ent;
+    int has_exec_key = 0;
+    QDict *input_dict;
+
+    if (qobject_type(input_obj) != QTYPE_QDICT) {
+        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "object");
+        return NULL;
+    }
+
+    input_dict = qobject_to_qdict(input_obj);
+
+    for (ent = qdict_first(input_dict); ent; ent = qdict_next(input_dict, ent)){
+        const char *arg_name = qdict_entry_key(ent);
+        const QObject *arg_obj = qdict_entry_value(ent);
+
+        if (!strcmp(arg_name, "execute")) {
+            if (qobject_type(arg_obj) != QTYPE_QSTRING) {
+                qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute",
+                              "string");
+                return NULL;
+            }
+            has_exec_key = 1;
+        } else if (!strcmp(arg_name, "arguments")) {
+            if (qobject_type(arg_obj) != QTYPE_QDICT) {
+                qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments",
+                              "object");
+                return NULL;
+            }
+        } else if (!strcmp(arg_name, "id")) {
+            /* FIXME: check duplicated IDs for async commands */
+        } else {
+            qerror_report(QERR_QMP_EXTRA_MEMBER, arg_name);
+            return NULL;
+        }
+    }
+
+    if (!has_exec_key) {
+        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "execute");
+        return NULL;
+    }
+
+    return input_dict;
+}
+
 static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
 {
     int err;
@@ -4184,7 +4240,11 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         goto err_out;
     }
 
-    input = qobject_to_qdict(obj);
+    input = qmp_check_input_obj(obj);
+    if (!input) {
+        qobject_decref(obj);
+        goto err_out;
+    }
 
     mon->mc->id = qdict_get(input, "id");
     qobject_incref(mon->mc->id);
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 13/14] QMP: Drop old input object checking

[Luiz Capitulino]

Previous commit added qmp_check_input_obj(), it does all the
checking we need.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   19 +------------------
 1 files changed, 1 insertions(+), 18 deletions(-)

diff --git a/monitor.c b/monitor.c
index fab553a..b68b464 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4234,10 +4234,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         // FIXME: should be triggered in json_parser_parse()
         qerror_report(QERR_JSON_PARSING);
         goto err_out;
-    } else if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "object");
-        qobject_decref(obj);
-        goto err_out;
     }
 
     input = qmp_check_input_obj(obj);
@@ -4249,17 +4245,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     mon->mc->id = qdict_get(input, "id");
     qobject_incref(mon->mc->id);
 
-    obj = qdict_get(input, "execute");
-    if (!obj) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT, "execute");
-        goto err_input;
-    } else if (qobject_type(obj) != QTYPE_QSTRING) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "execute", "string");
-        goto err_input;
-    }
-
-    cmd_name = qstring_get_str(qobject_to_qstring(obj));
-
+    cmd_name = qdict_get_str(input, "execute");
     if (invalid_qmp_mode(mon, cmd_name)) {
         qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
         goto err_input;
@@ -4287,9 +4273,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     obj = qdict_get(input, "arguments");
     if (!obj) {
         args = qdict_new();
-    } else if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, "arguments", "object");
-        goto err_input;
     } else {
         args = qobject_to_qdict(obj);
         QINCREF(args);
-- 
1.7.1.359.gd0b8d

[Qemu-devel] [PATCH 14/14] QMP: handle_qmp_command(): Small cleanup

[Luiz Capitulino]

Drop a unneeded label and QDECREF() call.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 monitor.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/monitor.c b/monitor.c
index b68b464..e38a3a4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4227,7 +4227,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     Monitor *mon = cur_mon;
     const char *cmd_name, *info_item;
 
-    args = NULL;
+    args = input = NULL;
 
     obj = json_parser_parse(tokens, NULL);
     if (!obj) {
@@ -4248,7 +4248,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     cmd_name = qdict_get_str(input, "execute");
     if (invalid_qmp_mode(mon, cmd_name)) {
         qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-        goto err_input;
+        goto err_out;
     }
 
     /*
@@ -4257,7 +4257,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
      */
     if (compare_cmd(cmd_name, "info")) {
         qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-        goto err_input;
+        goto err_out;
     } else if (strstart(cmd_name, "query-", &info_item)) {
         cmd = monitor_find_command("info");
         qdict_put_obj(input, "arguments",
@@ -4266,7 +4266,7 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         cmd = monitor_find_command(cmd_name);
         if (!cmd || !monitor_handler_ported(cmd)) {
             qerror_report(QERR_COMMAND_NOT_FOUND, cmd_name);
-            goto err_input;
+            goto err_out;
         }
     }
 
@@ -4278,8 +4278,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
         QINCREF(args);
     }
 
-    QDECREF(input);
-
     err = qmp_check_client_args(cmd, args);
     if (err < 0) {
         goto err_out;
@@ -4294,13 +4292,13 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens)
     } else {
         monitor_call_handler(mon, cmd, args);
     }
+
     goto out;
 
-err_input:
-    QDECREF(input);
 err_out:
     monitor_protocol_emitter(mon, NULL);
 out:
+    QDECREF(input);
     QDECREF(args);
 }
 
-- 
1.7.1.359.gd0b8d

Re: [Qemu-devel] [PATCH v3 00/14]: QMP: Replace client argument checker

[Luiz Capitulino]

On Thu, 24 Jun 2010 18:33:26 -0300
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> Current QMP's client argument checker code is more complex than it should be
> and has a flaw: it ignores unknown arguments.
> 
> This series solves both problems by introducing a new, simple and ultra-poweful
> argument checker. This wasn't trivial to get right due to the number of errors
> combinations, so review is very appreciated.
> 
> changelog
> ---------
> 
> v2 -> v3
> 
> - Move all input object checking into qmp_check_input_obj()
> - Fix remaining problem with the handling of O-type arguments
> - Small renames suggested by Markus
> - Small cleanup in handle_qmp_command()

Forgot to mention that changes from v2 begin in patch 08, and really relevant
ones in patch 12.

> 
> v1 -> v2
> 
> - Introduce new iteration API and use it
> - Handle O-type correctly (I hope so)
> - Address several small issues found by Markus
>