Re: [Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor
Il 02/04/2012 12:34, Laurent Desnogues ha scritto:
>> > static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
>> > {
>> > +GHashTableIter iter;
> GHashTableIter is alas not available in the glib (2.12) that
> the distros we use at work run. Is there a workaround for
> this issue?
Yeah, since the hash table is going to be destroyed anyway, and will
always be empty in the common case, you can call
g_hash_table_foreach_remove, and raise an error if it returns > 0
Something like
gboolean hash_table_empty_check(gpointer key, gpointer value,
gpointer user_data)
{
gpointer *p_key = user_data;
if (!*p_key) {
*p_key = key;
}
return true;
}
key = NULL;
if (g_hash_table_foreach_remove(qiv->stack[qiv->nb_stack - 1].h,
hash_table_empty_check, &key) {
error_set(errp, QERR_QMP_EXTRA_MEMBER, (char *) key);
}
g_hash_table_unref(qiv->stack[qiv->nb_stack - 1].h);
Can you make a patch?
Paolo
Re: [Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor
Hello,
On Thu, Mar 22, 2012 at 12:51 PM, Paolo Bonzini wrote:
> While QMP in general is designed so that it is possible to ignore
> unknown arguments, in the case of the QMP server it is better to
> reject them to detect bad clients. In fact, we're already doing
> this at the top level in the argument checker. To extend this to
> complex structures, add a mode to the input visitor where it checks
> for unvisited keys and raises an error if it finds one.
>
> Signed-off-by: Paolo Bonzini
> ---
> qapi/qmp-input-visitor.c | 48 +-
> qapi/qmp-input-visitor.h | 2 +
> test-qmp-input-strict.c | 234
> ++
> tests/Makefile | 5 +-
> 4 files changed, 285 insertions(+), 4 deletions(-)
> create mode 100644 test-qmp-input-strict.c
>
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index 97a0186..eb09874 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -24,6 +24,7 @@ typedef struct StackObject
> {
> QObject *obj;
> const QListEntry *entry;
> + GHashTable *h;
> } StackObject;
>
> struct QmpInputVisitor
> @@ -31,6 +32,7 @@ struct QmpInputVisitor
> Visitor visitor;
> StackObject stack[QIV_STACK_SIZE];
> int nb_stack;
> + bool strict;
> };
>
> static QmpInputVisitor *to_qiv(Visitor *v)
> @@ -45,6 +47,9 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
>
> if (qobj) {
> if (name && qobject_type(qobj) == QTYPE_QDICT) {
> + if (qiv->stack[qiv->nb_stack - 1].h) {
> + g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name);
> + }
> return qdict_get(qobject_to_qdict(qobj), name);
> } else if (qiv->stack[qiv->nb_stack - 1].entry) {
> return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
> @@ -54,20 +59,47 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
> return qobj;
> }
>
> +static void qdict_add_key(const char *key, QObject *obj, void *opaque)
> +{
> + GHashTable *h = opaque;
> + g_hash_table_insert(h, (gpointer) key, NULL);
> +}
> +
> static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
> {
> - qiv->stack[qiv->nb_stack].obj = obj;
> - qiv->stack[qiv->nb_stack].entry = NULL;
> - qiv->nb_stack++;
> + GHashTable *h;
>
> if (qiv->nb_stack >= QIV_STACK_SIZE) {
> error_set(errp, QERR_BUFFER_OVERRUN);
> return;
> }
> +
> + qiv->stack[qiv->nb_stack].obj = obj;
> + qiv->stack[qiv->nb_stack].entry = NULL;
> + qiv->stack[qiv->nb_stack].h = NULL;
> +
> + if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) {
> + h = g_hash_table_new(g_str_hash, g_str_equal);
> + qdict_iter(qobject_to_qdict(obj), qdict_add_key, h);
> + qiv->stack[qiv->nb_stack].h = h;
> + }
> +
> + qiv->nb_stack++;
> }
>
> static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
> {
> + GHashTableIter iter;
GHashTableIter is alas not available in the glib (2.12) that
the distros we use at work run. Is there a workaround for
this issue?
Thanks,
Laurent
> + gpointer key;
> +
> + if (qiv->strict && qiv->stack[qiv->nb_stack - 1].h) {
> + g_hash_table_iter_init(&iter, qiv->stack[qiv->nb_stack - 1].h);
> + if (g_hash_table_iter_next(&iter, &key, NULL)) {
> + error_set(errp, QERR_QMP_EXTRA_MEMBER, (char *) key);
> + }
> + g_hash_table_unref(qiv->stack[qiv->nb_stack - 1].h);
> + }
> +
> assert(qiv->nb_stack > 0);
> qiv->nb_stack--;
> }
> @@ -257,3 +289,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
>
> return v;
> }
> +
> +QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj)
> +{
> + QmpInputVisitor *v;
> +
> + v = qmp_input_visitor_new(obj);
> + v->strict = true;
> +
> + return v;
> +}
> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
> index 3f798f0..e0a48a5 100644
> --- a/qapi/qmp-input-visitor.h
> +++ b/qapi/qmp-input-visitor.h
> @@ -20,6 +20,8 @@
> typedef struct QmpInputVisitor QmpInputVisitor;
>
> QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
> +QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);
> +
> void qmp_input_visitor_cleanup(QmpInputVisitor *v);
>
> Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
> diff --git a/test-qmp-input-strict.c b/test-qmp-input-strict.c
> new file mode 100644
> index 000..f6df8cb
> --- /dev/null
> +++ b/test-qmp-input-strict.c
> @@ -0,0 +1,234 @@
> +/*
> + * QMP Input Visitor unit-tests (strict mode).
> + *
> + * Copyright (C) 2011-2012 Red Hat Inc.
> + *
> + * Authors:
> + * Luiz Capitulino
> + * Paolo Bonzini
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include
> +#include
> +
> +#include "qapi/qmp-input-visitor.h"
> +#include "test-qapi-types.h"
> +#include "test-qapi-visit.h"
> +#include "qe
Re: [Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor
On 03/22/2012 06:51 AM, Paolo Bonzini wrote:
While QMP in general is designed so that it is possible to ignore
unknown arguments, in the case of the QMP server it is better to
reject them to detect bad clients. In fact, we're already doing
this at the top level in the argument checker. To extend this to
complex structures, add a mode to the input visitor where it checks
for unvisited keys and raises an error if it finds one.
Signed-off-by: Paolo Bonzini
We need to document this behavior in QMP/qmp-spec.txt.
Regards,
Anthony Liguori
---
qapi/qmp-input-visitor.c | 48 +-
qapi/qmp-input-visitor.h |2 +
test-qmp-input-strict.c | 234 ++
tests/Makefile |5 +-
4 files changed, 285 insertions(+), 4 deletions(-)
create mode 100644 test-qmp-input-strict.c
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 97a0186..eb09874 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -24,6 +24,7 @@ typedef struct StackObject
{
QObject *obj;
const QListEntry *entry;
+GHashTable *h;
} StackObject;
struct QmpInputVisitor
@@ -31,6 +32,7 @@ struct QmpInputVisitor
Visitor visitor;
StackObject stack[QIV_STACK_SIZE];
int nb_stack;
+bool strict;
};
static QmpInputVisitor *to_qiv(Visitor *v)
@@ -45,6 +47,9 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
if (qobj) {
if (name&& qobject_type(qobj) == QTYPE_QDICT) {
+if (qiv->stack[qiv->nb_stack - 1].h) {
+g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name);
+}
return qdict_get(qobject_to_qdict(qobj), name);
} else if (qiv->stack[qiv->nb_stack - 1].entry) {
return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
@@ -54,20 +59,47 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
return qobj;
}
+static void qdict_add_key(const char *key, QObject *obj, void *opaque)
+{
+GHashTable *h = opaque;
+g_hash_table_insert(h, (gpointer) key, NULL);
+}
+
static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
{
-qiv->stack[qiv->nb_stack].obj = obj;
-qiv->stack[qiv->nb_stack].entry = NULL;
-qiv->nb_stack++;
+GHashTable *h;
if (qiv->nb_stack>= QIV_STACK_SIZE) {
error_set(errp, QERR_BUFFER_OVERRUN);
return;
}
+
+qiv->stack[qiv->nb_stack].obj = obj;
+qiv->stack[qiv->nb_stack].entry = NULL;
+qiv->stack[qiv->nb_stack].h = NULL;
+
+if (qiv->strict&& qobject_type(obj) == QTYPE_QDICT) {
+h = g_hash_table_new(g_str_hash, g_str_equal);
+qdict_iter(qobject_to_qdict(obj), qdict_add_key, h);
+qiv->stack[qiv->nb_stack].h = h;
+}
+
+qiv->nb_stack++;
}
static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
{
+GHashTableIter iter;
+gpointer key;
+
+if (qiv->strict&& qiv->stack[qiv->nb_stack - 1].h) {
+g_hash_table_iter_init(&iter, qiv->stack[qiv->nb_stack - 1].h);
+if (g_hash_table_iter_next(&iter,&key, NULL)) {
+error_set(errp, QERR_QMP_EXTRA_MEMBER, (char *) key);
+}
+g_hash_table_unref(qiv->stack[qiv->nb_stack - 1].h);
+}
+
assert(qiv->nb_stack> 0);
qiv->nb_stack--;
}
@@ -257,3 +289,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
return v;
}
+
+QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj)
+{
+QmpInputVisitor *v;
+
+v = qmp_input_visitor_new(obj);
+v->strict = true;
+
+return v;
+}
diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
index 3f798f0..e0a48a5 100644
--- a/qapi/qmp-input-visitor.h
+++ b/qapi/qmp-input-visitor.h
@@ -20,6 +20,8 @@
typedef struct QmpInputVisitor QmpInputVisitor;
QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
+QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);
+
void qmp_input_visitor_cleanup(QmpInputVisitor *v);
Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
diff --git a/test-qmp-input-strict.c b/test-qmp-input-strict.c
new file mode 100644
index 000..f6df8cb
--- /dev/null
+++ b/test-qmp-input-strict.c
@@ -0,0 +1,234 @@
+/*
+ * QMP Input Visitor unit-tests (strict mode).
+ *
+ * Copyright (C) 2011-2012 Red Hat Inc.
+ *
+ * Authors:
+ * Luiz Capitulino
+ * Paolo Bonzini
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include
+#include
+
+#include "qapi/qmp-input-visitor.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qemu-objects.h"
+
+typedef struct TestInputVisitorData {
+QObject *obj;
+QmpInputVisitor *qiv;
+} TestInputVisitorData;
+
+static void validate_teardown(TestInputVisitorData *data,
+ const void *unused)
+{
+qobject_decref(data->obj);
+data->obj = NULL;
+
+if (data->qiv) {
+
[Qemu-devel] [PATCH 08/10] qapi: add strict mode to input visitor
While QMP in general is designed so that it is possible to ignore
unknown arguments, in the case of the QMP server it is better to
reject them to detect bad clients. In fact, we're already doing
this at the top level in the argument checker. To extend this to
complex structures, add a mode to the input visitor where it checks
for unvisited keys and raises an error if it finds one.
Signed-off-by: Paolo Bonzini
---
qapi/qmp-input-visitor.c | 48 +-
qapi/qmp-input-visitor.h |2 +
test-qmp-input-strict.c | 234 ++
tests/Makefile |5 +-
4 files changed, 285 insertions(+), 4 deletions(-)
create mode 100644 test-qmp-input-strict.c
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 97a0186..eb09874 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -24,6 +24,7 @@ typedef struct StackObject
{
QObject *obj;
const QListEntry *entry;
+GHashTable *h;
} StackObject;
struct QmpInputVisitor
@@ -31,6 +32,7 @@ struct QmpInputVisitor
Visitor visitor;
StackObject stack[QIV_STACK_SIZE];
int nb_stack;
+bool strict;
};
static QmpInputVisitor *to_qiv(Visitor *v)
@@ -45,6 +47,9 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
if (qobj) {
if (name && qobject_type(qobj) == QTYPE_QDICT) {
+if (qiv->stack[qiv->nb_stack - 1].h) {
+g_hash_table_remove(qiv->stack[qiv->nb_stack - 1].h, name);
+}
return qdict_get(qobject_to_qdict(qobj), name);
} else if (qiv->stack[qiv->nb_stack - 1].entry) {
return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
@@ -54,20 +59,47 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
return qobj;
}
+static void qdict_add_key(const char *key, QObject *obj, void *opaque)
+{
+GHashTable *h = opaque;
+g_hash_table_insert(h, (gpointer) key, NULL);
+}
+
static void qmp_input_push(QmpInputVisitor *qiv, QObject *obj, Error **errp)
{
-qiv->stack[qiv->nb_stack].obj = obj;
-qiv->stack[qiv->nb_stack].entry = NULL;
-qiv->nb_stack++;
+GHashTable *h;
if (qiv->nb_stack >= QIV_STACK_SIZE) {
error_set(errp, QERR_BUFFER_OVERRUN);
return;
}
+
+qiv->stack[qiv->nb_stack].obj = obj;
+qiv->stack[qiv->nb_stack].entry = NULL;
+qiv->stack[qiv->nb_stack].h = NULL;
+
+if (qiv->strict && qobject_type(obj) == QTYPE_QDICT) {
+h = g_hash_table_new(g_str_hash, g_str_equal);
+qdict_iter(qobject_to_qdict(obj), qdict_add_key, h);
+qiv->stack[qiv->nb_stack].h = h;
+}
+
+qiv->nb_stack++;
}
static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
{
+GHashTableIter iter;
+gpointer key;
+
+if (qiv->strict && qiv->stack[qiv->nb_stack - 1].h) {
+g_hash_table_iter_init(&iter, qiv->stack[qiv->nb_stack - 1].h);
+if (g_hash_table_iter_next(&iter, &key, NULL)) {
+error_set(errp, QERR_QMP_EXTRA_MEMBER, (char *) key);
+}
+g_hash_table_unref(qiv->stack[qiv->nb_stack - 1].h);
+}
+
assert(qiv->nb_stack > 0);
qiv->nb_stack--;
}
@@ -257,3 +289,13 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
return v;
}
+
+QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj)
+{
+QmpInputVisitor *v;
+
+v = qmp_input_visitor_new(obj);
+v->strict = true;
+
+return v;
+}
diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
index 3f798f0..e0a48a5 100644
--- a/qapi/qmp-input-visitor.h
+++ b/qapi/qmp-input-visitor.h
@@ -20,6 +20,8 @@
typedef struct QmpInputVisitor QmpInputVisitor;
QmpInputVisitor *qmp_input_visitor_new(QObject *obj);
+QmpInputVisitor *qmp_input_visitor_new_strict(QObject *obj);
+
void qmp_input_visitor_cleanup(QmpInputVisitor *v);
Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
diff --git a/test-qmp-input-strict.c b/test-qmp-input-strict.c
new file mode 100644
index 000..f6df8cb
--- /dev/null
+++ b/test-qmp-input-strict.c
@@ -0,0 +1,234 @@
+/*
+ * QMP Input Visitor unit-tests (strict mode).
+ *
+ * Copyright (C) 2011-2012 Red Hat Inc.
+ *
+ * Authors:
+ * Luiz Capitulino
+ * Paolo Bonzini
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include
+#include
+
+#include "qapi/qmp-input-visitor.h"
+#include "test-qapi-types.h"
+#include "test-qapi-visit.h"
+#include "qemu-objects.h"
+
+typedef struct TestInputVisitorData {
+QObject *obj;
+QmpInputVisitor *qiv;
+} TestInputVisitorData;
+
+static void validate_teardown(TestInputVisitorData *data,
+ const void *unused)
+{
+qobject_decref(data->obj);
+data->obj = NULL;
+
+if (data->qiv) {
+qmp_input_visitor_cleanup(data->qiv);
+data->qiv = NULL;
+}
+}
+
+/* This is provided instead of a test setup function so that the JSON
+
