* [Qemu-devel] [PATCH 01/13] qapi: fix double free in qmp_output_visitor_cleanup()
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 02/13] qapi: add struct-errors test case to test-qmp-output-visitor Luiz Capitulino
` (12 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Laszlo Ersek, qemu-devel
From: Laszlo Ersek <lersek@redhat.com>
Stack entries in QmpOutputVisitor are navigation links (weak references),
except the bottom (ie. least recently added) entry, which owns the root
QObject [1]. Make qmp_output_visitor_cleanup() drop the stack entries,
then release the QObject tree by the root.
Attempting to serialize an invalid enum inside a dictionary is an example
for triggering the double free.
[1] http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg03276.html
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/qmp-output-visitor.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/qapi/qmp-output-visitor.c b/qapi/qmp-output-visitor.c
index e0697b0..2bce9d5 100644
--- a/qapi/qmp-output-visitor.c
+++ b/qapi/qmp-output-visitor.c
@@ -199,14 +199,16 @@ void qmp_output_visitor_cleanup(QmpOutputVisitor *v)
{
QStackEntry *e, *tmp;
+ /* The bottom QStackEntry, if any, owns the root QObject. See the
+ * qmp_output_push_obj() invocations in qmp_output_add_obj(). */
+ QObject *root = QTAILQ_EMPTY(&v->stack) ? NULL : qmp_output_first(v);
+
QTAILQ_FOREACH_SAFE(e, &v->stack, node, tmp) {
QTAILQ_REMOVE(&v->stack, e, node);
- if (e->value) {
- qobject_decref(e->value);
- }
g_free(e);
}
+ qobject_decref(root);
g_free(v);
}
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 02/13] qapi: add struct-errors test case to test-qmp-output-visitor
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 01/13] qapi: fix double free in qmp_output_visitor_cleanup() Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 03/13] qapi: add a test case for type errors Luiz Capitulino
` (11 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, Laszlo Ersek, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
This test case verifies that invalid native enums are caught, and causes
qapi to tear down the QObject tree under construction, exercising the
previous patch.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi-schema-test.json | 2 +-
test-qmp-output-visitor.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/qapi-schema-test.json b/qapi-schema-test.json
index 8c7f9f7..9eae350 100644
--- a/qapi-schema-test.json
+++ b/qapi-schema-test.json
@@ -8,7 +8,7 @@
# for testing nested structs
{ 'type': 'UserDefOne',
- 'data': { 'integer': 'int', 'string': 'str' } }
+ 'data': { 'integer': 'int', 'string': 'str', '*enum1': 'EnumOne' } }
{ 'type': 'UserDefTwo',
'data': { 'string': 'str',
diff --git a/test-qmp-output-visitor.c b/test-qmp-output-visitor.c
index 4d6c4d4..24a6359 100644
--- a/test-qmp-output-visitor.c
+++ b/test-qmp-output-visitor.c
@@ -274,6 +274,24 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
qapi_free_UserDefNested(ud2);
}
+static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
+ const void *unused)
+{
+ EnumOne bad_values[] = { ENUM_ONE_MAX, -1 };
+ UserDefOne u = { 0 }, *pu = &u;
+ Error *errp;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
+ errp = NULL;
+ u.has_enum1 = true;
+ u.enum1 = bad_values[i];
+ visit_type_UserDefOne(data->ov, &pu, "unused", &errp);
+ g_assert(error_is_set(&errp) == true);
+ error_free(errp);
+ }
+}
+
typedef struct TestStructList
{
TestStruct *value;
@@ -444,6 +462,8 @@ int main(int argc, char **argv)
&out_visitor_data, test_visitor_out_struct);
output_visitor_test_add("/visitor/output/struct-nested",
&out_visitor_data, test_visitor_out_struct_nested);
+ output_visitor_test_add("/visitor/output/struct-errors",
+ &out_visitor_data, test_visitor_out_struct_errors);
output_visitor_test_add("/visitor/output/list",
&out_visitor_data, test_visitor_out_list);
output_visitor_test_add("/visitor/output/list-qapi-free",
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 03/13] qapi: add a test case for type errors
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 01/13] qapi: fix double free in qmp_output_visitor_cleanup() Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 02/13] qapi: add struct-errors test case to test-qmp-output-visitor Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 04/13] qapi: fail hard on stack imbalance Luiz Capitulino
` (10 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
There is no test case for parse errors, add one.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
test-qmp-input-visitor.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/test-qmp-input-visitor.c b/test-qmp-input-visitor.c
index 1996e49..c30fdc4 100644
--- a/test-qmp-input-visitor.c
+++ b/test-qmp-input-visitor.c
@@ -258,6 +258,23 @@ static void input_visitor_test_add(const char *testpath,
visitor_input_teardown);
}
+static void test_visitor_in_errors(TestInputVisitorData *data,
+ const void *unused)
+{
+ TestStruct *p = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }");
+
+ visit_type_TestStruct(v, &p, NULL, &errp);
+ g_assert(error_is_set(&errp));
+ g_assert(p->string == NULL);
+
+ g_free(p->string);
+ g_free(p);
+}
+
int main(int argc, char **argv)
{
TestInputVisitorData in_visitor_data;
@@ -282,6 +299,8 @@ int main(int argc, char **argv)
&in_visitor_data, test_visitor_in_list);
input_visitor_test_add("/visitor/input/union",
&in_visitor_data, test_visitor_in_union);
+ input_visitor_test_add("/visitor/input/errors",
+ &in_visitor_data, test_visitor_in_errors);
g_test_run();
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 04/13] qapi: fail hard on stack imbalance
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (2 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 03/13] qapi: add a test case for type errors Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 05/13] qapi: fix memory leak on error Luiz Capitulino
` (9 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
QmpOutputVisitor will segfault if an imbalanced end function is
called. So we can abort in QmpInputVisitor too.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/qmp-input-visitor.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index e6b6152..b4013cc 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -77,11 +77,8 @@ static void qmp_input_push(QmpInputVisitor *qiv, const QObject *obj, Error **err
static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
{
+ assert(qiv->nb_stack > 0);
qiv->nb_stack--;
- if (qiv->nb_stack < 0) {
- error_set(errp, QERR_BUFFER_OVERRUN);
- return;
- }
}
static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 05/13] qapi: fix memory leak on error
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (3 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 04/13] qapi: fail hard on stack imbalance Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 06/13] qapi: shortcut visits on errors Luiz Capitulino
` (8 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
QmpInputVisitor would leak the malloced struct if the stack was
overflowed. This can be easily fixed using error_propagate.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/qmp-input-visitor.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index b4013cc..ef9288f 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -86,6 +86,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
{
QmpInputVisitor *qiv = to_qiv(v);
const QObject *qobj = qmp_input_get_object(qiv, name);
+ Error *err = NULL;
if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -93,8 +94,9 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
return;
}
- qmp_input_push(qiv, qobj, errp);
- if (error_is_set(errp)) {
+ qmp_input_push(qiv, qobj, &err);
+ if (err) {
+ error_propagate(errp, err);
return;
}
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 06/13] qapi: shortcut visits on errors
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (4 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 05/13] qapi: fix memory leak on error Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 07/13] qapi: allow freeing partially-allocated objects Luiz Capitulino
` (7 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
We can exit very soon if we enter a visitor with a preexisting error.
This simplifies some cases because we will not have to deal with
obj being non-NULL while *obj is NULL.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi-visit.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 78c947c..4297621 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -61,6 +61,9 @@ def generate_visit_struct(name, members):
void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp)
{
+ if (error_is_set(errp)) {
+ return;
+ }
visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), errp);
''',
name=name)
@@ -81,6 +84,9 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name,
{
GenericList *i, **head = (GenericList **)obj;
+ if (error_is_set(errp)) {
+ return;
+ }
visit_start_list(m, name, errp);
for (*head = i = visit_next_list(m, head, errp); i; i = visit_next_list(m, &i, errp)) {
@@ -112,6 +118,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
{
Error *err = NULL;
+ if (error_is_set(errp)) {
+ return;
+ }
visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err);
if (err) {
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 07/13] qapi: allow freeing partially-allocated objects
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (5 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 06/13] qapi: shortcut visits on errors Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 08/13] qapi: untangle next_list Luiz Capitulino
` (6 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Objects going through the dealloc visitor can be only partially allocated.
Detect the situation and avoid a segfault. This also helps with the
input visitor, when there are errors.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi-visit.py | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 4297621..31d50a6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -65,6 +65,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
return;
}
visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), errp);
+ if (obj && !*obj) {
+ goto end;
+ }
''',
name=name)
push_indent()
@@ -72,6 +75,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
pop_indent()
ret += mcgen('''
+end:
visit_end_struct(m, errp);
}
''')
@@ -122,6 +126,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
return;
}
visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+ if (obj && !*obj) {
+ goto end;
+ }
visit_type_%(name)sKind(m, &(*obj)->kind, "type", &err);
if (err) {
error_propagate(errp, err);
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 08/13] qapi: untangle next_list
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (6 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 07/13] qapi: allow freeing partially-allocated objects Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 09/13] qapi: place outermost object on qiv stack Luiz Capitulino
` (5 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Right now, the semantics of next_list are complicated. The caller must:
* call start_list
* call next_list for each element *including the first*
* on the first call to next_list, the second argument should point to
NULL and the result is the head of the list. On subsequent calls,
the second argument should point to the last node (last result of
next_list) and next_list itself tacks the element at the tail of the
list.
This works for both input and output visitor, but having the visitor
write memory when it is only reading the list is ugly. Plus, relying
on *list to detect the first call is tricky and undocumented.
We can initialize so->entry in next_list instead of start_list, leaving
it NULL in start_list. This way next_list sees clearly whether it is
on the first call---as a bonus, it discriminates the cases based on
internal state of the visitor rather than external state. We can
also pull the assignment of the list head from generated code up to
next_list.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
docs/qapi-code-gen.txt | 4 ++--
qapi/qmp-input-visitor.c | 22 +++++++++++++---------
scripts/qapi-visit.py | 4 ++--
3 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 5831e37..ad11767 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -194,11 +194,11 @@ Example:
void visit_type_UserDefOneList(Visitor *m, UserDefOneList ** obj, const char *name, Error **errp)
{
- GenericList *i;
+ GenericList *i, **prev = (GenericList **)obj;
visit_start_list(m, name, errp);
- for (i = visit_next_list(m, (GenericList **)obj, errp); i; i = visit_next_list(m, &i, errp)) {
+ for (; (i = visit_next_list(m, prev, errp)) != NULL; prev = &i) {
UserDefOneList *native_i = (UserDefOneList *)i;
visit_type_UserDefOne(m, &native_i->value, NULL, errp);
}
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index ef9288f..413e333 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -64,9 +64,7 @@ static const QObject *qmp_input_get_object(QmpInputVisitor *qiv,
static void qmp_input_push(QmpInputVisitor *qiv, const QObject *obj, Error **errp)
{
qiv->stack[qiv->nb_stack].obj = obj;
- if (qobject_type(obj) == QTYPE_QLIST) {
- qiv->stack[qiv->nb_stack].entry = qlist_first(qobject_to_qlist(obj));
- }
+ qiv->stack[qiv->nb_stack].entry = NULL;
qiv->nb_stack++;
if (qiv->nb_stack >= QIV_STACK_SIZE) {
@@ -132,18 +130,24 @@ static GenericList *qmp_input_next_list(Visitor *v, GenericList **list,
QmpInputVisitor *qiv = to_qiv(v);
GenericList *entry;
StackObject *so = &qiv->stack[qiv->nb_stack - 1];
+ bool first;
+
+ if (so->entry == NULL) {
+ so->entry = qlist_first(qobject_to_qlist(so->obj));
+ first = true;
+ } else {
+ so->entry = qlist_next(so->entry);
+ first = false;
+ }
if (so->entry == NULL) {
return NULL;
}
entry = g_malloc0(sizeof(*entry));
- if (*list) {
- so->entry = qlist_next(so->entry);
- if (so->entry == NULL) {
- g_free(entry);
- return NULL;
- }
+ if (first) {
+ *list = entry;
+ } else {
(*list)->next = entry;
}
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 31d50a6..8d4e94a 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -86,14 +86,14 @@ def generate_visit_list(name, members):
void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char *name, Error **errp)
{
- GenericList *i, **head = (GenericList **)obj;
+ GenericList *i, **prev = (GenericList **)obj;
if (error_is_set(errp)) {
return;
}
visit_start_list(m, name, errp);
- for (*head = i = visit_next_list(m, head, errp); i; i = visit_next_list(m, &i, errp)) {
+ for (; (i = visit_next_list(m, prev, errp)) != NULL; prev = &i) {
%(name)sList *native_i = (%(name)sList *)i;
visit_type_%(name)s(m, &native_i->value, NULL, errp);
}
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 09/13] qapi: place outermost object on qiv stack
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (7 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 08/13] qapi: untangle next_list Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 10/13] qapi: add strict mode to input visitor Luiz Capitulino
` (4 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
This is a slight change in the implementation of QMPInputVisitor
that helps when adding strict mode.
Const QObjects cannot be inc/decref-ed, and that's why QMPInputVisitor
relies heavily on weak references to inner objects. I'm not removing
the weak references now, but since refcount+const is a lost battle in C
(C++ has "mutable") I think removing const is fine in this case.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi/qmp-input-visitor.c | 41 +++++++++++++++++------------------------
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 413e333..5765e76 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -22,14 +22,13 @@
typedef struct StackObject
{
- const QObject *obj;
- const QListEntry *entry;
+ QObject *obj;
+ const QListEntry *entry;
} StackObject;
struct QmpInputVisitor
{
Visitor visitor;
- QObject *obj;
StackObject stack[QIV_STACK_SIZE];
int nb_stack;
};
@@ -39,21 +38,15 @@ static QmpInputVisitor *to_qiv(Visitor *v)
return container_of(v, QmpInputVisitor, visitor);
}
-static const QObject *qmp_input_get_object(QmpInputVisitor *qiv,
- const char *name)
+static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
+ const char *name)
{
- const QObject *qobj;
-
- if (qiv->nb_stack == 0) {
- qobj = qiv->obj;
- } else {
- qobj = qiv->stack[qiv->nb_stack - 1].obj;
- }
+ QObject *qobj = qiv->stack[qiv->nb_stack - 1].obj;
if (qobj) {
if (name && qobject_type(qobj) == QTYPE_QDICT) {
return qdict_get(qobject_to_qdict(qobj), name);
- } else if (qiv->nb_stack > 0 && qobject_type(qobj) == QTYPE_QLIST) {
+ } else if (qiv->stack[qiv->nb_stack - 1].entry) {
return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
}
}
@@ -61,7 +54,7 @@ static const QObject *qmp_input_get_object(QmpInputVisitor *qiv,
return qobj;
}
-static void qmp_input_push(QmpInputVisitor *qiv, const QObject *obj, Error **errp)
+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;
@@ -83,7 +76,7 @@ static void qmp_input_start_struct(Visitor *v, void **obj, const char *kind,
const char *name, size_t size, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- const QObject *qobj = qmp_input_get_object(qiv, name);
+ QObject *qobj = qmp_input_get_object(qiv, name);
Error *err = NULL;
if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
@@ -113,7 +106,7 @@ static void qmp_input_end_struct(Visitor *v, Error **errp)
static void qmp_input_start_list(Visitor *v, const char *name, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- const QObject *qobj = qmp_input_get_object(qiv, name);
+ QObject *qobj = qmp_input_get_object(qiv, name);
if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -165,7 +158,7 @@ static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- const QObject *qobj = qmp_input_get_object(qiv, name);
+ QObject *qobj = qmp_input_get_object(qiv, name);
if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -180,7 +173,7 @@ static void qmp_input_type_bool(Visitor *v, bool *obj, const char *name,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- const QObject *qobj = qmp_input_get_object(qiv, name);
+ QObject *qobj = qmp_input_get_object(qiv, name);
if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -195,7 +188,7 @@ static void qmp_input_type_str(Visitor *v, char **obj, const char *name,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- const QObject *qobj = qmp_input_get_object(qiv, name);
+ QObject *qobj = qmp_input_get_object(qiv, name);
if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -210,7 +203,7 @@ static void qmp_input_type_number(Visitor *v, double *obj, const char *name,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- const QObject *qobj = qmp_input_get_object(qiv, name);
+ QObject *qobj = qmp_input_get_object(qiv, name);
if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
@@ -225,7 +218,7 @@ static void qmp_input_start_optional(Visitor *v, bool *present,
const char *name, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
- const QObject *qobj = qmp_input_get_object(qiv, name);
+ QObject *qobj = qmp_input_get_object(qiv, name);
if (!qobj) {
*present = false;
@@ -242,7 +235,7 @@ Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
void qmp_input_visitor_cleanup(QmpInputVisitor *v)
{
- qobject_decref(v->obj);
+ qobject_decref(v->stack[0].obj);
g_free(v);
}
@@ -264,8 +257,8 @@ QmpInputVisitor *qmp_input_visitor_new(QObject *obj)
v->visitor.type_number = qmp_input_type_number;
v->visitor.start_optional = qmp_input_start_optional;
- v->obj = obj;
- qobject_incref(v->obj);
+ qmp_input_push(v, obj, NULL);
+ qobject_incref(obj);
return v;
}
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 10/13] qapi: add strict mode to input visitor
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (8 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 09/13] qapi: place outermost object on qiv stack Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 11/13] qmp: add and use q type specifier Luiz Capitulino
` (3 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
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 <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
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 5765e76..74386b9 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--;
}
@@ -262,3 +294,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 0000000..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 <lcapitulino@redhat.com>
+ * Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * 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 <glib.h>
+#include <stdarg.h>
+
+#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
+ string used by the tests are kept in the test functions (and not
+ int main()) */
+static GCC_FMT_ATTR(2, 3)
+Visitor *validate_test_init(TestInputVisitorData *data,
+ const char *json_string, ...)
+{
+ Visitor *v;
+ va_list ap;
+
+ va_start(ap, json_string);
+ data->obj = qobject_from_jsonv(json_string, &ap);
+ va_end(ap);
+
+ g_assert(data->obj != NULL);
+
+ data->qiv = qmp_input_visitor_new_strict(data->obj);
+ g_assert(data->qiv != NULL);
+
+ v = qmp_input_get_visitor(data->qiv);
+ g_assert(v != NULL);
+
+ return v;
+}
+
+typedef struct TestStruct
+{
+ int64_t integer;
+ bool boolean;
+ char *string;
+} TestStruct;
+
+static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
+ const char *name, Error **errp)
+{
+ visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
+ errp);
+
+ visit_type_int(v, &(*obj)->integer, "integer", errp);
+ visit_type_bool(v, &(*obj)->boolean, "boolean", errp);
+ visit_type_str(v, &(*obj)->string, "string", errp);
+
+ visit_end_struct(v, errp);
+}
+
+static void test_validate_struct(TestInputVisitorData *data,
+ const void *unused)
+{
+ TestStruct *p = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");
+
+ visit_type_TestStruct(v, &p, NULL, &errp);
+ g_assert(!error_is_set(&errp));
+ g_free(p->string);
+ g_free(p);
+}
+
+static void test_validate_struct_nested(TestInputVisitorData *data,
+ const void *unused)
+{
+ UserDefNested *udp = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string' }, 'string2': 'string2'}}}");
+
+ visit_type_UserDefNested(v, &udp, NULL, &errp);
+ g_assert(!error_is_set(&errp));
+ qapi_free_UserDefNested(udp);
+}
+
+static void test_validate_list(TestInputVisitorData *data,
+ const void *unused)
+{
+ UserDefOneList *head = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]");
+
+ visit_type_UserDefOneList(v, &head, NULL, &errp);
+ g_assert(!error_is_set(&errp));
+ qapi_free_UserDefOneList(head);
+}
+
+static void test_validate_union(TestInputVisitorData *data,
+ const void *unused)
+{
+ UserDefUnion *tmp = NULL;
+ Visitor *v;
+ Error *errp = NULL;
+
+ v = validate_test_init(data, "{ 'type': 'b', 'data' : { 'integer': 42 } }");
+
+ visit_type_UserDefUnion(v, &tmp, NULL, &errp);
+ g_assert(!error_is_set(&errp));
+ qapi_free_UserDefUnion(tmp);
+}
+
+static void test_validate_fail_struct(TestInputVisitorData *data,
+ const void *unused)
+{
+ TestStruct *p = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }");
+
+ visit_type_TestStruct(v, &p, NULL, &errp);
+ g_assert(error_is_set(&errp));
+ if (p) {
+ g_free(p->string);
+ }
+ g_free(p);
+}
+
+static void test_validate_fail_struct_nested(TestInputVisitorData *data,
+ const void *unused)
+{
+ UserDefNested *udp = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}");
+
+ visit_type_UserDefNested(v, &udp, NULL, &errp);
+ g_assert(error_is_set(&errp));
+ qapi_free_UserDefNested(udp);
+}
+
+static void test_validate_fail_list(TestInputVisitorData *data,
+ const void *unused)
+{
+ UserDefOneList *head = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]");
+
+ visit_type_UserDefOneList(v, &head, NULL, &errp);
+ g_assert(error_is_set(&errp));
+ qapi_free_UserDefOneList(head);
+}
+
+static void test_validate_fail_union(TestInputVisitorData *data,
+ const void *unused)
+{
+ UserDefUnion *tmp = NULL;
+ Error *errp = NULL;
+ Visitor *v;
+
+ v = validate_test_init(data, "{ 'type': 'b', 'data' : { 'integer': 42 }, 'extra': 'yyy' }");
+
+ visit_type_UserDefUnion(v, &tmp, NULL, &errp);
+ g_assert(error_is_set(&errp));
+ qapi_free_UserDefUnion(tmp);
+}
+
+static void validate_test_add(const char *testpath,
+ TestInputVisitorData *data,
+ void (*test_func)(TestInputVisitorData *data, const void *user_data))
+{
+ g_test_add(testpath, TestInputVisitorData, data, NULL, test_func,
+ validate_teardown);
+}
+
+int main(int argc, char **argv)
+{
+ TestInputVisitorData testdata;
+
+ g_test_init(&argc, &argv, NULL);
+
+ validate_test_add("/visitor/input-strict/pass/struct",
+ &testdata, test_validate_struct);
+ validate_test_add("/visitor/input-strict/pass/struct-nested",
+ &testdata, test_validate_struct_nested);
+ validate_test_add("/visitor/input-strict/pass/list",
+ &testdata, test_validate_list);
+ validate_test_add("/visitor/input-strict/pass/union",
+ &testdata, test_validate_union);
+ validate_test_add("/visitor/input-strict/fail/struct",
+ &testdata, test_validate_fail_struct);
+ validate_test_add("/visitor/input-strict/fail/struct-nested",
+ &testdata, test_validate_fail_struct_nested);
+ validate_test_add("/visitor/input-strict/fail/list",
+ &testdata, test_validate_fail_list);
+ validate_test_add("/visitor/input-strict/fail/union",
+ &testdata, test_validate_fail_union);
+
+ g_test_run();
+
+ return 0;
+}
diff --git a/tests/Makefile b/tests/Makefile
index 94ea342..2a2fff7 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -16,7 +16,7 @@ check-qfloat: check-qfloat.o qfloat.o $(tools-obj-y)
check-qjson: check-qjson.o $(qobject-obj-y) $(tools-obj-y)
test-coroutine: test-coroutine.o qemu-timer-common.o async.o $(coroutine-obj-y) $(tools-obj-y)
-test-qmp-input-visitor.o test-qmp-output-visitor.o \
+test-qmp-input-visitor.o test-qmp-output-visitor.o test-qmp-input-strict.o \
test-string-input-visitor.o test-string-output-visitor.o \
test-qmp-commands.o: QEMU_CFLAGS += -I $(qapi-dir)
@@ -37,6 +37,9 @@ test-string-output-visitor: test-string-output-visitor.o $(qobject-obj-y) $(qapi
test-string-input-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
test-string-input-visitor: test-string-input-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
+test-qmp-input-strict.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
+test-qmp-input-strict: test-qmp-input-strict.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
+
test-qmp-output-visitor.o: $(addprefix $(qapi-dir)/, test-qapi-types.c test-qapi-types.h test-qapi-visit.c test-qapi-visit.h) $(qapi-obj-y)
test-qmp-output-visitor: test-qmp-output-visitor.o $(qobject-obj-y) $(qapi-obj-y) $(tools-obj-y) $(qapi-dir)/test-qapi-visit.o $(qapi-dir)/test-qapi-types.o
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 11/13] qmp: add and use q type specifier
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (9 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 10/13] qapi: add strict mode to input visitor Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 12/13] qmp: parse commands in strict mode Luiz Capitulino
` (2 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
"O" is being used by the transaction and qom-set commands to mean "any
QObject", but it really means "do not validate the argument list".
Add a new specifier with the correct meaning.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
monitor.c | 3 +++
qmp-commands.hx | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/monitor.c b/monitor.c
index 2ff1e0b..8946a10 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4157,6 +4157,9 @@ static int check_client_args_type(const QDict *client_args,
case 'O':
assert(flags & QMP_ACCEPT_UNKNOWNS);
break;
+ case 'q':
+ /* Any QObject can be passed. */
+ break;
case '/':
case '.':
/*
diff --git a/qmp-commands.hx b/qmp-commands.hx
index c626ba8..9447871 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -708,7 +708,7 @@ EQMP
},
{
.name = "transaction",
- .args_type = "actions:O",
+ .args_type = "actions:q",
.mhandler.cmd_new = qmp_marshal_input_transaction,
},
@@ -2125,7 +2125,7 @@ EQMP
{
.name = "qom-set",
- .args_type = "path:s,property:s,opts:O",
+ .args_type = "path:s,property:s,value:q",
.mhandler.cmd_new = qmp_qom_set,
},
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 12/13] qmp: parse commands in strict mode
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (10 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 11/13] qmp: add and use q type specifier Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-27 12:20 ` [Qemu-devel] [PATCH 13/13] qmp: document strict parsing Luiz Capitulino
2012-03-30 13:12 ` [Qemu-devel] [PULL 00/13]: QMP queue Anthony Liguori
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi-commands.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 30a24d2..0b4f0a0 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -140,7 +140,7 @@ v = qapi_dealloc_get_visitor(md);
''')
else:
ret += mcgen('''
-mi = qmp_input_visitor_new(%(obj)s);
+mi = qmp_input_visitor_new_strict(%(obj)s);
v = qmp_input_get_visitor(mi);
''',
obj=obj)
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH 13/13] qmp: document strict parsing
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (11 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 12/13] qmp: parse commands in strict mode Luiz Capitulino
@ 2012-03-27 12:20 ` Luiz Capitulino
2012-03-30 13:12 ` [Qemu-devel] [PULL 00/13]: QMP queue Anthony Liguori
13 siblings, 0 replies; 21+ messages in thread
From: Luiz Capitulino @ 2012-03-27 12:20 UTC (permalink / raw)
To: aliguori; +Cc: Paolo Bonzini, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
QMP/qmp-spec.txt | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 9d30a8c..1ba916c 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -209,13 +209,27 @@ incompatible way are disabled by default and will be advertised by the
capabilities array (section '2.2 Server Greeting'). Thus, Clients can check
that array and enable the capabilities they support.
-Additionally, Clients must not assume any particular:
-
-- Size of json-objects or length of json-arrays
+The QMP Server performs a type check on the arguments to a command. It
+generates an error if a value does not have the expected type for its
+key, or if it does not understand a key that the Client included. The
+strictness of the Server catches wrong assumptions of Clients about
+the Server's schema. Clients can assume that, when such validation
+errors occur, they will be reported before the command generated any
+side effect.
+
+However, Clients must not assume any particular:
+
+- Length of json-arrays
+- Size of json-objects; in particular, future versions of QEMU may add
+ new keys and Clients should be able to ignore them.
- Order of json-object members or json-array elements
- Amount of errors generated by a command, that is, new errors can be added
to any existing command in newer versions of the Server
+Of course, the Server does guarantee to send valid JSON. But apart from
+this, a Client should be "conservative in what they send, and liberal in
+what they accept".
+
6. Downstream extension of QMP
------------------------------
--
1.7.9.2.384.g4a92a
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PULL 00/13]: QMP queue
2012-03-27 12:20 [Qemu-devel] [PULL 00/13]: QMP queue Luiz Capitulino
` (12 preceding siblings ...)
2012-03-27 12:20 ` [Qemu-devel] [PATCH 13/13] qmp: document strict parsing Luiz Capitulino
@ 2012-03-30 13:12 ` Anthony Liguori
13 siblings, 0 replies; 21+ messages in thread
From: Anthony Liguori @ 2012-03-30 13:12 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
On 03/27/2012 07:20 AM, Luiz Capitulino wrote:
> All of them are QAPI fixes.
>
> The changes (since 8a22565b7c2d1920b02b94e7a8021c65895a3a22) are available
> in the following repository:
>
> git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
Pulled. Thanks.
Regards,
Anthony Liguori
>
> Laszlo Ersek (1):
> qapi: fix double free in qmp_output_visitor_cleanup()
>
> Paolo Bonzini (12):
> qapi: add struct-errors test case to test-qmp-output-visitor
> qapi: add a test case for type errors
> qapi: fail hard on stack imbalance
> qapi: fix memory leak on error
> qapi: shortcut visits on errors
> qapi: allow freeing partially-allocated objects
> qapi: untangle next_list
> qapi: place outermost object on qiv stack
> qapi: add strict mode to input visitor
> qmp: add and use q type specifier
> qmp: parse commands in strict mode
> qmp: document strict parsing
>
> QMP/qmp-spec.txt | 20 +++-
> docs/qapi-code-gen.txt | 4 +-
> monitor.c | 3 +
> qapi-schema-test.json | 2 +-
> qapi/qmp-input-visitor.c | 120 +++++++++++++++--------
> qapi/qmp-input-visitor.h | 2 +
> qapi/qmp-output-visitor.c | 8 +-
> qmp-commands.hx | 4 +-
> scripts/qapi-commands.py | 2 +-
> scripts/qapi-visit.py | 20 +++-
> test-qmp-input-strict.c | 234 +++++++++++++++++++++++++++++++++++++++++++++
> test-qmp-input-visitor.c | 19 ++++
> test-qmp-output-visitor.c | 20 ++++
> tests/Makefile | 5 +-
> 14 files changed, 407 insertions(+), 56 deletions(-)
>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread