[Qemu-devel] [PATCH 0/4] qapi: Loose ends

[Qemu-devel] [PATCH 0/4] qapi: Loose ends

[Markus Armbruster]

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

[Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages

[Markus Armbruster]

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

Re: [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages

[Marc-André Lureau]

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

Re: [Qemu-devel] [PATCH 1/4] qmp: Improve QMP dispatch error messages

[Philippe Mathieu-Daudé]

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;
>      }
>
>

[Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth()

[Markus Armbruster]

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

Re: [Qemu-devel] [PATCH 2/4] qobject-input-visitor: Document full_name_nth()

[Marc-André Lureau]

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

[Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits

[Markus Armbruster]

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

Re: [Qemu-devel] [PATCH 3/4] qapi: Document intended use of @name within alternate visits

[Marc-André Lureau]

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

[Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list

[Markus Armbruster]

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

Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list

[Marc-André Lureau]

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

Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list

[Markus Armbruster]

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

Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list

[Marc-André Lureau]

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

Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list

[Markus Armbruster]

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
>>

Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list

[Eric Blake]

[-- 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 --]

Re: [Qemu-devel] [PATCH 4/4] qobject-input-visitor: Catch misuse of end_struct vs. end_list

[Markus Armbruster]

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);
 }