* [Qemu-devel] [PATCH 0/4] qapi: Loose ends
@ 2017-04-27 8:41 Markus Armbruster
2017-04-27 8:41 ` [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages Markus Armbruster
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 8:41 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, mdroth
This series addresses a few minor things that came up in review, but
too late to be addressed in 2.9.
Markus Armbruster (4):
qmp: Improve QMP dispatch error messages
qobject-input-visitor: Document full_name_nth()
qapi: Document intended use of @name within alternate visits
qobject-input-visitor: Catch misuse of end_struct vs. end_list
include/qapi/visitor.h | 6 ++++--
qapi/qmp-dispatch.c | 14 +++++++-------
qapi/qobject-input-visitor.c | 32 ++++++++++++++++++++++++++++++--
3 files changed, 41 insertions(+), 11 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages
2017-04-27 8:41 [Qemu-devel] [PATCH 0/4] qapi: Loose ends Markus Armbruster
@ 2017-04-27 8:41 ` Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
2017-05-02 13:14 ` Philippe Mathieu-Daudé
2017-04-27 8:41 ` [Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth() Markus Armbruster
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 8:41 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/qmp-dispatch.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index dc50212..5ad36f8 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -30,7 +30,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
dict = qobject_to_qdict(request);
if (!dict) {
- error_setg(errp, "Expected '%s' in QMP input", "object");
+ error_setg(errp, "QMP input must be a JSON object");
return NULL;
}
@@ -41,26 +41,26 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
if (!strcmp(arg_name, "execute")) {
if (qobject_type(arg_obj) != QTYPE_QSTRING) {
- error_setg(errp, "QMP input object member '%s' expects '%s'",
- "execute", "string");
+ error_setg(errp,
+ "QMP input member 'execute' must be a string");
return NULL;
}
has_exec_key = true;
} else if (!strcmp(arg_name, "arguments")) {
if (qobject_type(arg_obj) != QTYPE_QDICT) {
- error_setg(errp, "QMP input object member '%s' expects '%s'",
- "arguments", "object");
+ error_setg(errp,
+ "QMP input member 'arguments' must be an object");
return NULL;
}
} else {
- error_setg(errp, "QMP input object member '%s' is unexpected",
+ error_setg(errp, "QMP input member '%s' is unexpected",
arg_name);
return NULL;
}
}
if (!has_exec_key) {
- error_setg(errp, "Expected '%s' in QMP input", "execute");
+ error_setg(errp, "QMP input lacks member 'execute'");
return NULL;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth()
2017-04-27 8:41 [Qemu-devel] [PATCH 0/4] qapi: Loose ends Markus Armbruster
2017-04-27 8:41 ` [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages Markus Armbruster
@ 2017-04-27 8:41 ` Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
2017-04-27 8:41 ` [Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits Markus Armbruster
2017-04-27 8:41 ` [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list Markus Armbruster
3 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 8:41 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/qobject-input-visitor.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 865e948..2530959 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -55,6 +55,17 @@ static QObjectInputVisitor *to_qiv(Visitor *v)
return container_of(v, QObjectInputVisitor, visitor);
}
+/*
+ * Find the full name of something @qiv is currently visiting.
+ * @qiv is visiting something named @name in the stack of containers
+ * @qiv->stack.
+ * If @n is zero, return its full name.
+ * If @n is positive, return the full name of the @n-th container
+ * counting from the top. The stack of containers must have at least
+ * @n elements.
+ * The returned string is valid until the next full_name_nth(@v) or
+ * destruction of @v.
+ */
static const char *full_name_nth(QObjectInputVisitor *qiv, const char *name,
int n)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits
2017-04-27 8:41 [Qemu-devel] [PATCH 0/4] qapi: Loose ends Markus Armbruster
2017-04-27 8:41 ` [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages Markus Armbruster
2017-04-27 8:41 ` [Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth() Markus Armbruster
@ 2017-04-27 8:41 ` Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
2017-04-27 8:41 ` [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list Markus Armbruster
3 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 8:41 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/visitor.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 1a1b620..b0e233d 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -63,8 +63,10 @@
* The @name parameter of visit_type_FOO() describes the relation
* between this QAPI value and its parent container. When visiting
* the root of a tree, @name is ignored; when visiting a member of an
- * object, @name is the key associated with the value; and when
- * visiting a member of a list, @name is NULL.
+ * object, @name is the key associated with the value; when visiting a
+ * member of a list, @name is NULL; and when visiting the member of an
+ * alternate, @name should equal the name used for visiting the
+ * alternate.
*
* The visit_type_FOO() functions expect a non-null @obj argument;
* they allocate *@obj during input visits, leave it unchanged on
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list
2017-04-27 8:41 [Qemu-devel] [PATCH 0/4] qapi: Loose ends Markus Armbruster
` (2 preceding siblings ...)
2017-04-27 8:41 ` [Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits Markus Armbruster
@ 2017-04-27 8:41 ` Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
3 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 8:41 UTC (permalink / raw)
To: qemu-devel; +Cc: eblake, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/qobject-input-visitor.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 2530959..68a6742 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -291,6 +291,15 @@ static void qobject_input_start_struct(Visitor *v, const char *name, void **obj,
}
}
+static void qobject_input_end_struct(Visitor *v, void **obj)
+{
+ QObjectInputVisitor *qiv = to_qiv(v);
+ StackObject *tos = QSLIST_FIRST(&qiv->stack);
+
+ assert(tos->h);
+ qobject_input_pop(v, obj);
+}
+
static void qobject_input_start_list(Visitor *v, const char *name,
GenericList **list, size_t size,
@@ -346,6 +355,14 @@ static void qobject_input_check_list(Visitor *v, Error **errp)
}
}
+static void qobject_input_end_list(Visitor *v, void **obj)
+{
+ QObjectInputVisitor *qiv = to_qiv(v);
+ StackObject *tos = QSLIST_FIRST(&qiv->stack);
+
+ assert(!tos->h);
+ qobject_input_pop(v, obj);
+}
static void qobject_input_start_alternate(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
@@ -645,11 +662,11 @@ static QObjectInputVisitor *qobject_input_visitor_base_new(QObject *obj)
v->visitor.type = VISITOR_INPUT;
v->visitor.start_struct = qobject_input_start_struct;
v->visitor.check_struct = qobject_input_check_struct;
- v->visitor.end_struct = qobject_input_pop;
+ v->visitor.end_struct = qobject_input_end_struct;
v->visitor.start_list = qobject_input_start_list;
v->visitor.next_list = qobject_input_next_list;
v->visitor.check_list = qobject_input_check_list;
- v->visitor.end_list = qobject_input_pop;
+ v->visitor.end_list = qobject_input_end_list;
v->visitor.start_alternate = qobject_input_start_alternate;
v->visitor.optional = qobject_input_optional;
v->visitor.free = qobject_input_free;
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list
2017-04-27 8:41 ` [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list Markus Armbruster
@ 2017-04-27 9:06 ` Marc-André Lureau
2017-04-27 10:49 ` Markus Armbruster
0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2017-04-27 9:06 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On Thu, Apr 27, 2017 at 12:41 PM Markus Armbruster <armbru@redhat.com>
wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
It would be more obvious with a check for qobject_type(tos->obj), no?
In any case:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qapi/qobject-input-visitor.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 2530959..68a6742 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -291,6 +291,15 @@ static void qobject_input_start_struct(Visitor *v,
> const char *name, void **obj,
> }
> }
>
> +static void qobject_input_end_struct(Visitor *v, void **obj)
> +{
> + QObjectInputVisitor *qiv = to_qiv(v);
> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
> +
> + assert(tos->h);
> + qobject_input_pop(v, obj);
> +}
> +
>
> static void qobject_input_start_list(Visitor *v, const char *name,
> GenericList **list, size_t size,
> @@ -346,6 +355,14 @@ static void qobject_input_check_list(Visitor *v,
> Error **errp)
> }
> }
>
> +static void qobject_input_end_list(Visitor *v, void **obj)
> +{
> + QObjectInputVisitor *qiv = to_qiv(v);
> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
> +
> + assert(!tos->h);
> + qobject_input_pop(v, obj);
> +}
>
> static void qobject_input_start_alternate(Visitor *v, const char *name,
> GenericAlternate **obj, size_t
> size,
> @@ -645,11 +662,11 @@ static QObjectInputVisitor
> *qobject_input_visitor_base_new(QObject *obj)
> v->visitor.type = VISITOR_INPUT;
> v->visitor.start_struct = qobject_input_start_struct;
> v->visitor.check_struct = qobject_input_check_struct;
> - v->visitor.end_struct = qobject_input_pop;
> + v->visitor.end_struct = qobject_input_end_struct;
> v->visitor.start_list = qobject_input_start_list;
> v->visitor.next_list = qobject_input_next_list;
> v->visitor.check_list = qobject_input_check_list;
> - v->visitor.end_list = qobject_input_pop;
> + v->visitor.end_list = qobject_input_end_list;
> v->visitor.start_alternate = qobject_input_start_alternate;
> v->visitor.optional = qobject_input_optional;
> v->visitor.free = qobject_input_free;
> --
> 2.7.4
>
>
> --
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits
2017-04-27 8:41 ` [Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits Markus Armbruster
@ 2017-04-27 9:06 ` Marc-André Lureau
0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2017-04-27 9:06 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On Thu, Apr 27, 2017 at 12:42 PM Markus Armbruster <armbru@redhat.com>
wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> include/qapi/visitor.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 1a1b620..b0e233d 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -63,8 +63,10 @@
> * The @name parameter of visit_type_FOO() describes the relation
> * between this QAPI value and its parent container. When visiting
> * the root of a tree, @name is ignored; when visiting a member of an
> - * object, @name is the key associated with the value; and when
> - * visiting a member of a list, @name is NULL.
> + * object, @name is the key associated with the value; when visiting a
> + * member of a list, @name is NULL; and when visiting the member of an
> + * alternate, @name should equal the name used for visiting the
> + * alternate.
> *
> * The visit_type_FOO() functions expect a non-null @obj argument;
> * they allocate *@obj during input visits, leave it unchanged on
> --
> 2.7.4
>
>
> --
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth()
2017-04-27 8:41 ` [Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth() Markus Armbruster
@ 2017-04-27 9:06 ` Marc-André Lureau
0 siblings, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2017-04-27 9:06 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On Thu, Apr 27, 2017 at 12:41 PM Markus Armbruster <armbru@redhat.com>
wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qapi/qobject-input-visitor.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 865e948..2530959 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -55,6 +55,17 @@ static QObjectInputVisitor *to_qiv(Visitor *v)
> return container_of(v, QObjectInputVisitor, visitor);
> }
>
> +/*
> + * Find the full name of something @qiv is currently visiting.
> + * @qiv is visiting something named @name in the stack of containers
> + * @qiv->stack.
> + * If @n is zero, return its full name.
> + * If @n is positive, return the full name of the @n-th container
> + * counting from the top. The stack of containers must have at least
> + * @n elements.
> + * The returned string is valid until the next full_name_nth(@v) or
> + * destruction of @v.
> + */
> static const char *full_name_nth(QObjectInputVisitor *qiv, const char
> *name,
> int n)
> {
> --
> 2.7.4
>
>
> --
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages
2017-04-27 8:41 ` [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages Markus Armbruster
@ 2017-04-27 9:06 ` Marc-André Lureau
2017-05-02 13:14 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 15+ messages in thread
From: Marc-André Lureau @ 2017-04-27 9:06 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On Thu, Apr 27, 2017 at 12:46 PM Markus Armbruster <armbru@redhat.com>
wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> qapi/qmp-dispatch.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index dc50212..5ad36f8 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -30,7 +30,7 @@ static QDict *qmp_dispatch_check_obj(const QObject
> *request, Error **errp)
>
> dict = qobject_to_qdict(request);
> if (!dict) {
> - error_setg(errp, "Expected '%s' in QMP input", "object");
> + error_setg(errp, "QMP input must be a JSON object");
> return NULL;
> }
>
> @@ -41,26 +41,26 @@ static QDict *qmp_dispatch_check_obj(const QObject
> *request, Error **errp)
>
> if (!strcmp(arg_name, "execute")) {
> if (qobject_type(arg_obj) != QTYPE_QSTRING) {
> - error_setg(errp, "QMP input object member '%s' expects
> '%s'",
> - "execute", "string");
> + error_setg(errp,
> + "QMP input member 'execute' must be a string");
> return NULL;
> }
> has_exec_key = true;
> } else if (!strcmp(arg_name, "arguments")) {
> if (qobject_type(arg_obj) != QTYPE_QDICT) {
> - error_setg(errp, "QMP input object member '%s' expects
> '%s'",
> - "arguments", "object");
> + error_setg(errp,
> + "QMP input member 'arguments' must be an
> object");
> return NULL;
> }
> } else {
> - error_setg(errp, "QMP input object member '%s' is unexpected",
> + error_setg(errp, "QMP input member '%s' is unexpected",
> arg_name);
> return NULL;
> }
> }
>
> if (!has_exec_key) {
> - error_setg(errp, "Expected '%s' in QMP input", "execute");
> + error_setg(errp, "QMP input lacks member 'execute'");
> return NULL;
> }
>
> --
> 2.7.4
>
>
> --
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list
2017-04-27 9:06 ` Marc-André Lureau
@ 2017-04-27 10:49 ` Markus Armbruster
2017-04-27 10:56 ` Marc-André Lureau
0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 10:49 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, mdroth
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> On Thu, Apr 27, 2017 at 12:41 PM Markus Armbruster <armbru@redhat.com>
> wrote:
>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>
> It would be more obvious with a check for qobject_type(tos->obj), no?
--verbose?
> In any case:
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Thanks!
>> ---
>> qapi/qobject-input-visitor.c | 21 +++++++++++++++++++--
>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
>> index 2530959..68a6742 100644
>> --- a/qapi/qobject-input-visitor.c
>> +++ b/qapi/qobject-input-visitor.c
>> @@ -291,6 +291,15 @@ static void qobject_input_start_struct(Visitor *v,
>> const char *name, void **obj,
>> }
>> }
>>
>> +static void qobject_input_end_struct(Visitor *v, void **obj)
>> +{
>> + QObjectInputVisitor *qiv = to_qiv(v);
>> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
>> +
>> + assert(tos->h);
>> + qobject_input_pop(v, obj);
>> +}
>> +
>>
>> static void qobject_input_start_list(Visitor *v, const char *name,
>> GenericList **list, size_t size,
>> @@ -346,6 +355,14 @@ static void qobject_input_check_list(Visitor *v,
>> Error **errp)
>> }
>> }
>>
>> +static void qobject_input_end_list(Visitor *v, void **obj)
>> +{
>> + QObjectInputVisitor *qiv = to_qiv(v);
>> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
>> +
>> + assert(!tos->h);
>> + qobject_input_pop(v, obj);
>> +}
>>
>> static void qobject_input_start_alternate(Visitor *v, const char *name,
>> GenericAlternate **obj, size_t
>> size,
>> @@ -645,11 +662,11 @@ static QObjectInputVisitor
>> *qobject_input_visitor_base_new(QObject *obj)
>> v->visitor.type = VISITOR_INPUT;
>> v->visitor.start_struct = qobject_input_start_struct;
>> v->visitor.check_struct = qobject_input_check_struct;
>> - v->visitor.end_struct = qobject_input_pop;
>> + v->visitor.end_struct = qobject_input_end_struct;
>> v->visitor.start_list = qobject_input_start_list;
>> v->visitor.next_list = qobject_input_next_list;
>> v->visitor.check_list = qobject_input_check_list;
>> - v->visitor.end_list = qobject_input_pop;
>> + v->visitor.end_list = qobject_input_end_list;
>> v->visitor.start_alternate = qobject_input_start_alternate;
>> v->visitor.optional = qobject_input_optional;
>> v->visitor.free = qobject_input_free;
>> --
>> 2.7.4
>>
>>
>> --
> Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list
2017-04-27 10:49 ` Markus Armbruster
@ 2017-04-27 10:56 ` Marc-André Lureau
2017-04-27 11:50 ` Markus Armbruster
0 siblings, 1 reply; 15+ messages in thread
From: Marc-André Lureau @ 2017-04-27 10:56 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, mdroth
On Thu, Apr 27, 2017 at 2:49 PM Markus Armbruster <armbru@redhat.com> wrote:
> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>
> > On Thu, Apr 27, 2017 at 12:41 PM Markus Armbruster <armbru@redhat.com>
> > wrote:
> >
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >>
> >
> > It would be more obvious with a check for qobject_type(tos->obj), no?
>
> --verbose?
>
Instead of assert(tos->h) assert(qobject_type(tos->obj) == QTYPE_QDICT) ?
similarly for the list case.
>
> > In any case:
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Thanks!
>
> >> ---
> >> qapi/qobject-input-visitor.c | 21 +++++++++++++++++++--
> >> 1 file changed, 19 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> >> index 2530959..68a6742 100644
> >> --- a/qapi/qobject-input-visitor.c
> >> +++ b/qapi/qobject-input-visitor.c
> >> @@ -291,6 +291,15 @@ static void qobject_input_start_struct(Visitor *v,
> >> const char *name, void **obj,
> >> }
> >> }
> >>
> >> +static void qobject_input_end_struct(Visitor *v, void **obj)
> >> +{
> >> + QObjectInputVisitor *qiv = to_qiv(v);
> >> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
> >> +
> >> + assert(tos->h);
> >> + qobject_input_pop(v, obj);
> >> +}
> >> +
> >>
> >> static void qobject_input_start_list(Visitor *v, const char *name,
> >> GenericList **list, size_t size,
> >> @@ -346,6 +355,14 @@ static void qobject_input_check_list(Visitor *v,
> >> Error **errp)
> >> }
> >> }
> >>
> >> +static void qobject_input_end_list(Visitor *v, void **obj)
> >> +{
> >> + QObjectInputVisitor *qiv = to_qiv(v);
> >> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
> >> +
> >> + assert(!tos->h);
> >> + qobject_input_pop(v, obj);
> >> +}
> >>
> >> static void qobject_input_start_alternate(Visitor *v, const char *name,
> >> GenericAlternate **obj,
> size_t
> >> size,
> >> @@ -645,11 +662,11 @@ static QObjectInputVisitor
> >> *qobject_input_visitor_base_new(QObject *obj)
> >> v->visitor.type = VISITOR_INPUT;
> >> v->visitor.start_struct = qobject_input_start_struct;
> >> v->visitor.check_struct = qobject_input_check_struct;
> >> - v->visitor.end_struct = qobject_input_pop;
> >> + v->visitor.end_struct = qobject_input_end_struct;
> >> v->visitor.start_list = qobject_input_start_list;
> >> v->visitor.next_list = qobject_input_next_list;
> >> v->visitor.check_list = qobject_input_check_list;
> >> - v->visitor.end_list = qobject_input_pop;
> >> + v->visitor.end_list = qobject_input_end_list;
> >> v->visitor.start_alternate = qobject_input_start_alternate;
> >> v->visitor.optional = qobject_input_optional;
> >> v->visitor.free = qobject_input_free;
> >> --
> >> 2.7.4
> >>
> >>
> >> --
> > Marc-André Lureau
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list
2017-04-27 10:56 ` Marc-André Lureau
@ 2017-04-27 11:50 ` Markus Armbruster
2017-04-27 13:13 ` Eric Blake
0 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 11:50 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, mdroth, Eric Blake
Marc-André Lureau <marcandre.lureau@gmail.com> writes:
> On Thu, Apr 27, 2017 at 2:49 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Marc-André Lureau <marcandre.lureau@gmail.com> writes:
>>
>> > On Thu, Apr 27, 2017 at 12:41 PM Markus Armbruster <armbru@redhat.com>
>> > wrote:
>> >
>> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> >>
>> >
>> > It would be more obvious with a check for qobject_type(tos->obj), no?
>>
>> --verbose?
>>
>
> Instead of assert(tos->h) assert(qobject_type(tos->obj) == QTYPE_QDICT) ?
The two conditions imply each other. We can assert either one, or even
both. Eric, got a preference?
> similarly for the list case.
>
>> > In any case:
>> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Thanks!
>>
>> >> ---
>> >> qapi/qobject-input-visitor.c | 21 +++++++++++++++++++--
>> >> 1 file changed, 19 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
>> >> index 2530959..68a6742 100644
>> >> --- a/qapi/qobject-input-visitor.c
>> >> +++ b/qapi/qobject-input-visitor.c
>> >> @@ -291,6 +291,15 @@ static void qobject_input_start_struct(Visitor *v,
>> >> const char *name, void **obj,
>> >> }
>> >> }
>> >>
>> >> +static void qobject_input_end_struct(Visitor *v, void **obj)
>> >> +{
>> >> + QObjectInputVisitor *qiv = to_qiv(v);
>> >> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
>> >> +
>> >> + assert(tos->h);
>> >> + qobject_input_pop(v, obj);
>> >> +}
>> >> +
>> >>
>> >> static void qobject_input_start_list(Visitor *v, const char *name,
>> >> GenericList **list, size_t size,
>> >> @@ -346,6 +355,14 @@ static void qobject_input_check_list(Visitor *v,
>> >> Error **errp)
>> >> }
>> >> }
>> >>
>> >> +static void qobject_input_end_list(Visitor *v, void **obj)
>> >> +{
>> >> + QObjectInputVisitor *qiv = to_qiv(v);
>> >> + StackObject *tos = QSLIST_FIRST(&qiv->stack);
>> >> +
>> >> + assert(!tos->h);
>> >> + qobject_input_pop(v, obj);
>> >> +}
>> >>
>> >> static void qobject_input_start_alternate(Visitor *v, const char *name,
>> >> GenericAlternate **obj, size_t
>> >> size,
>> >> @@ -645,11 +662,11 @@ static QObjectInputVisitor
>> >> *qobject_input_visitor_base_new(QObject *obj)
>> >> v->visitor.type = VISITOR_INPUT;
>> >> v->visitor.start_struct = qobject_input_start_struct;
>> >> v->visitor.check_struct = qobject_input_check_struct;
>> >> - v->visitor.end_struct = qobject_input_pop;
>> >> + v->visitor.end_struct = qobject_input_end_struct;
>> >> v->visitor.start_list = qobject_input_start_list;
>> >> v->visitor.next_list = qobject_input_next_list;
>> >> v->visitor.check_list = qobject_input_check_list;
>> >> - v->visitor.end_list = qobject_input_pop;
>> >> + v->visitor.end_list = qobject_input_end_list;
>> >> v->visitor.start_alternate = qobject_input_start_alternate;
>> >> v->visitor.optional = qobject_input_optional;
>> >> v->visitor.free = qobject_input_free;
>> >> --
>> >> 2.7.4
>> >>
>> >>
>> >> --
>> > Marc-André Lureau
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list
2017-04-27 11:50 ` Markus Armbruster
@ 2017-04-27 13:13 ` Eric Blake
2017-04-27 14:02 ` Markus Armbruster
0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2017-04-27 13:13 UTC (permalink / raw)
To: Markus Armbruster, Marc-André Lureau; +Cc: qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 586 bytes --]
On 04/27/2017 06:50 AM, Markus Armbruster wrote:
>>>>
>>>> It would be more obvious with a check for qobject_type(tos->obj), no?
>>>
>>> --verbose?
>>>
>>
>> Instead of assert(tos->h) assert(qobject_type(tos->obj) == QTYPE_QDICT) ?
>
> The two conditions imply each other. We can assert either one, or even
> both. Eric, got a preference?
Asserting both might make it easier to come up to speed when reading the
code a year from now.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list
2017-04-27 13:13 ` Eric Blake
@ 2017-04-27 14:02 ` Markus Armbruster
0 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2017-04-27 14:02 UTC (permalink / raw)
To: Eric Blake; +Cc: Marc-André Lureau, qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 04/27/2017 06:50 AM, Markus Armbruster wrote:
>
>>>>>
>>>>> It would be more obvious with a check for qobject_type(tos->obj), no?
>>>>
>>>> --verbose?
>>>>
>>>
>>> Instead of assert(tos->h) assert(qobject_type(tos->obj) == QTYPE_QDICT) ?
>>
>> The two conditions imply each other. We can assert either one, or even
>> both. Eric, got a preference?
>
> Asserting both might make it easier to come up to speed when reading the
> code a year from now.
All right, squashing this:
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 68a6742..d0f0002 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -296,7 +296,7 @@ static void qobject_input_end_struct(Visitor *v, void **obj)
QObjectInputVisitor *qiv = to_qiv(v);
StackObject *tos = QSLIST_FIRST(&qiv->stack);
- assert(tos->h);
+ assert(qobject_type(tos->obj) == QTYPE_QDICT && tos->h);
qobject_input_pop(v, obj);
}
@@ -360,7 +360,7 @@ static void qobject_input_end_list(Visitor *v, void **obj)
QObjectInputVisitor *qiv = to_qiv(v);
StackObject *tos = QSLIST_FIRST(&qiv->stack);
- assert(!tos->h);
+ assert(qobject_type(tos->obj) == QTYPE_QLIST && !tos->h);
qobject_input_pop(v, obj);
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages
2017-04-27 8:41 ` [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
@ 2017-05-02 13:14 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-05-02 13:14 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: mdroth
On 04/27/2017 05:41 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> qapi/qmp-dispatch.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
> index dc50212..5ad36f8 100644
> --- a/qapi/qmp-dispatch.c
> +++ b/qapi/qmp-dispatch.c
> @@ -30,7 +30,7 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
>
> dict = qobject_to_qdict(request);
> if (!dict) {
> - error_setg(errp, "Expected '%s' in QMP input", "object");
> + error_setg(errp, "QMP input must be a JSON object");
> return NULL;
> }
>
> @@ -41,26 +41,26 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
>
> if (!strcmp(arg_name, "execute")) {
> if (qobject_type(arg_obj) != QTYPE_QSTRING) {
> - error_setg(errp, "QMP input object member '%s' expects '%s'",
> - "execute", "string");
> + error_setg(errp,
> + "QMP input member 'execute' must be a string");
> return NULL;
> }
> has_exec_key = true;
> } else if (!strcmp(arg_name, "arguments")) {
> if (qobject_type(arg_obj) != QTYPE_QDICT) {
> - error_setg(errp, "QMP input object member '%s' expects '%s'",
> - "arguments", "object");
> + error_setg(errp,
> + "QMP input member 'arguments' must be an object");
> return NULL;
> }
> } else {
> - error_setg(errp, "QMP input object member '%s' is unexpected",
> + error_setg(errp, "QMP input member '%s' is unexpected",
> arg_name);
> return NULL;
> }
> }
>
> if (!has_exec_key) {
> - error_setg(errp, "Expected '%s' in QMP input", "execute");
> + error_setg(errp, "QMP input lacks member 'execute'");
> return NULL;
> }
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-05-02 13:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-27 8:41 [Qemu-devel] [PATCH 0/4] qapi: Loose ends Markus Armbruster
2017-04-27 8:41 ` [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
2017-05-02 13:14 ` Philippe Mathieu-Daudé
2017-04-27 8:41 ` [Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth() Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
2017-04-27 8:41 ` [Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
2017-04-27 8:41 ` [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list Markus Armbruster
2017-04-27 9:06 ` Marc-André Lureau
2017-04-27 10:49 ` Markus Armbruster
2017-04-27 10:56 ` Marc-André Lureau
2017-04-27 11:50 ` Markus Armbruster
2017-04-27 13:13 ` Eric Blake
2017-04-27 14:02 ` Markus Armbruster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.