Patchwork [v5,05/18] qapi: add QMP input visitor

login
register
mail settings
Submitter Michael Roth
Date July 5, 2011, 1:02 p.m.
Message ID <1309870965-27066-6-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/103287/
State New
Headers show

Comments

Michael Roth - July 5, 2011, 1:02 p.m.
A type of Visiter class that is used to walk a qobject's
structure and assign each entry to the corresponding native C type.
Command marshaling function will use this to pull out QMP command
parameters recieved over the wire and pass them as native arguments
to the corresponding C functions.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs            |    2 +-
 qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
 qapi/qmp-input-visitor.h |   27 +++++
 qerror.h                 |    3 +
 4 files changed, 295 insertions(+), 1 deletions(-)
 create mode 100644 qapi/qmp-input-visitor.c
 create mode 100644 qapi/qmp-input-visitor.h
Luiz Capitulino - July 7, 2011, 2:32 p.m.
On Tue,  5 Jul 2011 08:02:32 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> A type of Visiter class that is used to walk a qobject's
> structure and assign each entry to the corresponding native C type.
> Command marshaling function will use this to pull out QMP command
> parameters recieved over the wire and pass them as native arguments
> to the corresponding C functions.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  Makefile.objs            |    2 +-
>  qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/qmp-input-visitor.h |   27 +++++
>  qerror.h                 |    3 +
>  4 files changed, 295 insertions(+), 1 deletions(-)
>  create mode 100644 qapi/qmp-input-visitor.c
>  create mode 100644 qapi/qmp-input-visitor.h
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index 0077014..997ecef 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
>  ######################################################################
>  # qapi
>  
> -qapi-nested-y = qapi-visit-core.o
> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
>  qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>  
>  vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> new file mode 100644
> index 0000000..80912bb
> --- /dev/null
> +++ b/qapi/qmp-input-visitor.c
> @@ -0,0 +1,264 @@
> +/*
> + * Input Visitor
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori   <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "qmp-input-visitor.h"
> +#include "qemu-queue.h"
> +#include "qemu-common.h"
> +#include "qemu-objects.h"
> +#include "qerror.h"
> +
> +#define QIV_STACK_SIZE 1024
> +
> +typedef struct StackObject
> +{
> +    const QObject *obj;
> +    const  QListEntry *entry;
> +} StackObject;
> +
> +struct QmpInputVisitor
> +{
> +    Visitor visitor;
> +    const QObject *obj;
> +    StackObject stack[QIV_STACK_SIZE];
> +    int nb_stack;
> +};
> +
> +static QmpInputVisitor *to_qiv(Visitor *v)
> +{
> +    return container_of(v, QmpInputVisitor, visitor);
> +}
> +
> +static const 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;
> +    }
> +
> +    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) {
> +        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
> +    }
> +
> +    return qobj;
> +}
> +
> +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->nb_stack++;
> +
> +    if (qiv->nb_stack >= QIV_STACK_SIZE) {
> +        error_set(errp, QERR_BUFFER_OVERRUN);
> +        return;
> +    }
> +}
> +
> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
> +{
> +    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, const char *name, size_t size, Error **errp)
> +{
> +    QmpInputVisitor *qiv = to_qiv(v);
> +    const QObject *qobj = qmp_input_get_object(qiv, name);
> +
> +    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
> +        return;
> +    }
> +
> +    qmp_input_push(qiv, qobj, errp);
> +    if (error_is_set(errp)) {
> +        return;
> +    }
> +
> +    if (obj) {
> +        *obj = qemu_mallocz(size);
> +    }
> +}
> +
> +static void qmp_input_end_struct(Visitor *v, Error **errp)
> +{
> +    QmpInputVisitor *qiv = to_qiv(v);
> +
> +    qmp_input_pop(qiv, 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);
> +
> +    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
> +        return;
> +    }
> +
> +    qmp_input_push(qiv, qobj, errp);
> +}
> +
> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
> +{
> +    QmpInputVisitor *qiv = to_qiv(v);
> +    GenericList *entry;
> +    StackObject *so = &qiv->stack[qiv->nb_stack - 1];
> +
> +    if (so->entry == NULL) {
> +        return NULL;
> +    }
> +
> +    entry = qemu_mallocz(sizeof(*entry));
> +    if (*list) {
> +        so->entry = qlist_next(so->entry);
> +        if (so->entry == NULL) {
> +            qemu_free(entry);
> +            return NULL;
> +        }
> +        (*list)->next = entry;
> +    }
> +    *list = entry;
> +
> +
> +    return entry;
> +}
> +
> +static void qmp_input_end_list(Visitor *v, Error **errp)
> +{
> +    QmpInputVisitor *qiv = to_qiv(v);
> +
> +    qmp_input_pop(qiv, errp);
> +}
> +
> +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);
> +
> +    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
> +        return;
> +    }
> +
> +    *obj = qint_get_int(qobject_to_qint(qobj));
> +}
> +
> +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);
> +
> +    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
> +        return;
> +    }
> +
> +    *obj = qbool_get_int(qobject_to_qbool(qobj));
> +}
> +
> +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);
> +
> +    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
> +        return;
> +    }
> +
> +    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
> +}
> +
> +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);
> +
> +    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
> +        return;
> +    }
> +
> +    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
> +}
> +
> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
> +{
> +    int64_t value;
> +    qmp_input_type_int(v, &value, name, errp);
> +    *obj = value;
> +}
> +
> +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);
> +
> +    if (!qobj) {
> +        *present = false;
> +        return;
> +    }
> +
> +    *present = true;
> +}
> +
> +static void qmp_input_end_optional(Visitor *v, Error **errp)
> +{
> +}
> +
> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
> +{
> +    return &v->visitor;
> +}
> +
> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
> +{

We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and
decrefing here.

> +    qemu_free(v);
> +}
> +
> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
> +{
> +    QmpInputVisitor *v;
> +
> +    v = qemu_mallocz(sizeof(*v));
> +
> +    v->visitor.start_struct = qmp_input_start_struct;
> +    v->visitor.end_struct = qmp_input_end_struct;
> +    v->visitor.start_list = qmp_input_start_list;
> +    v->visitor.next_list = qmp_input_next_list;
> +    v->visitor.end_list = qmp_input_end_list;
> +    v->visitor.type_enum = qmp_input_type_enum;
> +    v->visitor.type_int = qmp_input_type_int;
> +    v->visitor.type_bool = qmp_input_type_bool;
> +    v->visitor.type_str = qmp_input_type_str;
> +    v->visitor.type_number = qmp_input_type_number;
> +    v->visitor.start_optional = qmp_input_start_optional;
> +    v->visitor.end_optional = qmp_input_end_optional;
> +
> +    v->obj = obj;
> +
> +    return v;
> +}
> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
> new file mode 100644
> index 0000000..0f77916
> --- /dev/null
> +++ b/qapi/qmp-input-visitor.h
> @@ -0,0 +1,27 @@
> +/*
> + * Input Visitor
> + *
> + * Copyright IBM, Corp. 2011
> + *
> + * Authors:
> + *  Anthony Liguori   <aliguori@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#ifndef QMP_INPUT_VISITOR_H
> +#define QMP_INPUT_VISITOR_H
> +
> +#include "qapi-visit-core.h"
> +#include "qobject.h"
> +
> +typedef struct QmpInputVisitor QmpInputVisitor;
> +
> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
> +void qmp_input_visitor_cleanup(QmpInputVisitor *v);
> +
> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
> +
> +#endif
> diff --git a/qerror.h b/qerror.h
> index 16c830d..9a9fa5b 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj);
>  #define QERR_JSON_PARSE_ERROR \
>      "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
>  
> +#define QERR_BUFFER_OVERRUN \
> +    "{ 'class': 'BufferOverrun', 'data': {} }"
> +

You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory,
but BufferOverrrun is probably fine.

>  #define QERR_KVM_MISSING_CAP \
>      "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
>
Michael Roth - July 7, 2011, 2:45 p.m.
On 07/07/2011 09:32 AM, Luiz Capitulino wrote:
> On Tue,  5 Jul 2011 08:02:32 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> A type of Visiter class that is used to walk a qobject's
>> structure and assign each entry to the corresponding native C type.
>> Command marshaling function will use this to pull out QMP command
>> parameters recieved over the wire and pass them as native arguments
>> to the corresponding C functions.
>>
>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>> ---
>>   Makefile.objs            |    2 +-
>>   qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
>>   qapi/qmp-input-visitor.h |   27 +++++
>>   qerror.h                 |    3 +
>>   4 files changed, 295 insertions(+), 1 deletions(-)
>>   create mode 100644 qapi/qmp-input-visitor.c
>>   create mode 100644 qapi/qmp-input-visitor.h
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 0077014..997ecef 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
>>   ######################################################################
>>   # qapi
>>
>> -qapi-nested-y = qapi-visit-core.o
>> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
>>   qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>>
>>   vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
>> new file mode 100644
>> index 0000000..80912bb
>> --- /dev/null
>> +++ b/qapi/qmp-input-visitor.c
>> @@ -0,0 +1,264 @@
>> +/*
>> + * Input Visitor
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + *  Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qmp-input-visitor.h"
>> +#include "qemu-queue.h"
>> +#include "qemu-common.h"
>> +#include "qemu-objects.h"
>> +#include "qerror.h"
>> +
>> +#define QIV_STACK_SIZE 1024
>> +
>> +typedef struct StackObject
>> +{
>> +    const QObject *obj;
>> +    const  QListEntry *entry;
>> +} StackObject;
>> +
>> +struct QmpInputVisitor
>> +{
>> +    Visitor visitor;
>> +    const QObject *obj;
>> +    StackObject stack[QIV_STACK_SIZE];
>> +    int nb_stack;
>> +};
>> +
>> +static QmpInputVisitor *to_qiv(Visitor *v)
>> +{
>> +    return container_of(v, QmpInputVisitor, visitor);
>> +}
>> +
>> +static const 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;
>> +    }
>> +
>> +    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) {
>> +        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
>> +    }
>> +
>> +    return qobj;
>> +}
>> +
>> +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->nb_stack++;
>> +
>> +    if (qiv->nb_stack>= QIV_STACK_SIZE) {
>> +        error_set(errp, QERR_BUFFER_OVERRUN);
>> +        return;
>> +    }
>> +}
>> +
>> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
>> +{
>> +    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, const char *name, size_t size, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +    const QObject *qobj = qmp_input_get_object(qiv, name);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
>> +        return;
>> +    }
>> +
>> +    qmp_input_push(qiv, qobj, errp);
>> +    if (error_is_set(errp)) {
>> +        return;
>> +    }
>> +
>> +    if (obj) {
>> +        *obj = qemu_mallocz(size);
>> +    }
>> +}
>> +
>> +static void qmp_input_end_struct(Visitor *v, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +
>> +    qmp_input_pop(qiv, 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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
>> +        return;
>> +    }
>> +
>> +    qmp_input_push(qiv, qobj, errp);
>> +}
>> +
>> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +    GenericList *entry;
>> +    StackObject *so =&qiv->stack[qiv->nb_stack - 1];
>> +
>> +    if (so->entry == NULL) {
>> +        return NULL;
>> +    }
>> +
>> +    entry = qemu_mallocz(sizeof(*entry));
>> +    if (*list) {
>> +        so->entry = qlist_next(so->entry);
>> +        if (so->entry == NULL) {
>> +            qemu_free(entry);
>> +            return NULL;
>> +        }
>> +        (*list)->next = entry;
>> +    }
>> +    *list = entry;
>> +
>> +
>> +    return entry;
>> +}
>> +
>> +static void qmp_input_end_list(Visitor *v, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +
>> +    qmp_input_pop(qiv, errp);
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
>> +        return;
>> +    }
>> +
>> +    *obj = qint_get_int(qobject_to_qint(qobj));
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
>> +        return;
>> +    }
>> +
>> +    *obj = qbool_get_int(qobject_to_qbool(qobj));
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
>> +        return;
>> +    }
>> +
>> +    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
>> +        return;
>> +    }
>> +
>> +    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
>> +}
>> +
>> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
>> +{
>> +    int64_t value;
>> +    qmp_input_type_int(v,&value, name, errp);
>> +    *obj = value;
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj) {
>> +        *present = false;
>> +        return;
>> +    }
>> +
>> +    *present = true;
>> +}
>> +
>> +static void qmp_input_end_optional(Visitor *v, Error **errp)
>> +{
>> +}
>> +
>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
>> +{
>> +    return&v->visitor;
>> +}
>> +
>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
>> +{
>
> We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and
> decrefing here.
>

Sorry, remember the discussion but must've missed it when addressing 
comments. Will add it.

>> +    qemu_free(v);
>> +}
>> +
>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
>> +{
>> +    QmpInputVisitor *v;
>> +
>> +    v = qemu_mallocz(sizeof(*v));
>> +
>> +    v->visitor.start_struct = qmp_input_start_struct;
>> +    v->visitor.end_struct = qmp_input_end_struct;
>> +    v->visitor.start_list = qmp_input_start_list;
>> +    v->visitor.next_list = qmp_input_next_list;
>> +    v->visitor.end_list = qmp_input_end_list;
>> +    v->visitor.type_enum = qmp_input_type_enum;
>> +    v->visitor.type_int = qmp_input_type_int;
>> +    v->visitor.type_bool = qmp_input_type_bool;
>> +    v->visitor.type_str = qmp_input_type_str;
>> +    v->visitor.type_number = qmp_input_type_number;
>> +    v->visitor.start_optional = qmp_input_start_optional;
>> +    v->visitor.end_optional = qmp_input_end_optional;
>> +
>> +    v->obj = obj;
>> +
>> +    return v;
>> +}
>> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
>> new file mode 100644
>> index 0000000..0f77916
>> --- /dev/null
>> +++ b/qapi/qmp-input-visitor.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Input Visitor
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + *  Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef QMP_INPUT_VISITOR_H
>> +#define QMP_INPUT_VISITOR_H
>> +
>> +#include "qapi-visit-core.h"
>> +#include "qobject.h"
>> +
>> +typedef struct QmpInputVisitor QmpInputVisitor;
>> +
>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v);
>> +
>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
>> +
>> +#endif
>> diff --git a/qerror.h b/qerror.h
>> index 16c830d..9a9fa5b 100644
>> --- a/qerror.h
>> +++ b/qerror.h
>> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj);
>>   #define QERR_JSON_PARSE_ERROR \
>>       "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
>>
>> +#define QERR_BUFFER_OVERRUN \
>> +    "{ 'class': 'BufferOverrun', 'data': {} }"
>> +
>
> You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory,
> but BufferOverrrun is probably fine.
>

I like overrun since it doesn't suggest an issue with insufficient 
host/process/guest memory. Will add to qerror.c

>>   #define QERR_KVM_MISSING_CAP \
>>       "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
>>
>
Michael Roth - July 12, 2011, 12:05 a.m.
On 07/07/2011 09:32 AM, Luiz Capitulino wrote:
> On Tue,  5 Jul 2011 08:02:32 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> A type of Visiter class that is used to walk a qobject's
>> structure and assign each entry to the corresponding native C type.
>> Command marshaling function will use this to pull out QMP command
>> parameters recieved over the wire and pass them as native arguments
>> to the corresponding C functions.
>>
>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>> ---
>>   Makefile.objs            |    2 +-
>>   qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
>>   qapi/qmp-input-visitor.h |   27 +++++
>>   qerror.h                 |    3 +
>>   4 files changed, 295 insertions(+), 1 deletions(-)
>>   create mode 100644 qapi/qmp-input-visitor.c
>>   create mode 100644 qapi/qmp-input-visitor.h
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 0077014..997ecef 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
>>   ######################################################################
>>   # qapi
>>
>> -qapi-nested-y = qapi-visit-core.o
>> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
>>   qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>>
>>   vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
>> new file mode 100644
>> index 0000000..80912bb
>> --- /dev/null
>> +++ b/qapi/qmp-input-visitor.c
>> @@ -0,0 +1,264 @@
>> +/*
>> + * Input Visitor
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + *  Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qmp-input-visitor.h"
>> +#include "qemu-queue.h"
>> +#include "qemu-common.h"
>> +#include "qemu-objects.h"
>> +#include "qerror.h"
>> +
>> +#define QIV_STACK_SIZE 1024
>> +
>> +typedef struct StackObject
>> +{
>> +    const QObject *obj;
>> +    const  QListEntry *entry;
>> +} StackObject;
>> +
>> +struct QmpInputVisitor
>> +{
>> +    Visitor visitor;
>> +    const QObject *obj;
>> +    StackObject stack[QIV_STACK_SIZE];
>> +    int nb_stack;
>> +};
>> +
>> +static QmpInputVisitor *to_qiv(Visitor *v)
>> +{
>> +    return container_of(v, QmpInputVisitor, visitor);
>> +}
>> +
>> +static const 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;
>> +    }
>> +
>> +    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) {
>> +        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
>> +    }
>> +
>> +    return qobj;
>> +}
>> +
>> +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->nb_stack++;
>> +
>> +    if (qiv->nb_stack>= QIV_STACK_SIZE) {
>> +        error_set(errp, QERR_BUFFER_OVERRUN);
>> +        return;
>> +    }
>> +}
>> +
>> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
>> +{
>> +    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, const char *name, size_t size, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +    const QObject *qobj = qmp_input_get_object(qiv, name);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
>> +        return;
>> +    }
>> +
>> +    qmp_input_push(qiv, qobj, errp);
>> +    if (error_is_set(errp)) {
>> +        return;
>> +    }
>> +
>> +    if (obj) {
>> +        *obj = qemu_mallocz(size);
>> +    }
>> +}
>> +
>> +static void qmp_input_end_struct(Visitor *v, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +
>> +    qmp_input_pop(qiv, 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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
>> +        return;
>> +    }
>> +
>> +    qmp_input_push(qiv, qobj, errp);
>> +}
>> +
>> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +    GenericList *entry;
>> +    StackObject *so =&qiv->stack[qiv->nb_stack - 1];
>> +
>> +    if (so->entry == NULL) {
>> +        return NULL;
>> +    }
>> +
>> +    entry = qemu_mallocz(sizeof(*entry));
>> +    if (*list) {
>> +        so->entry = qlist_next(so->entry);
>> +        if (so->entry == NULL) {
>> +            qemu_free(entry);
>> +            return NULL;
>> +        }
>> +        (*list)->next = entry;
>> +    }
>> +    *list = entry;
>> +
>> +
>> +    return entry;
>> +}
>> +
>> +static void qmp_input_end_list(Visitor *v, Error **errp)
>> +{
>> +    QmpInputVisitor *qiv = to_qiv(v);
>> +
>> +    qmp_input_pop(qiv, errp);
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
>> +        return;
>> +    }
>> +
>> +    *obj = qint_get_int(qobject_to_qint(qobj));
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
>> +        return;
>> +    }
>> +
>> +    *obj = qbool_get_int(qobject_to_qbool(qobj));
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
>> +        return;
>> +    }
>> +
>> +    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
>> +        return;
>> +    }
>> +
>> +    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
>> +}
>> +
>> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
>> +{
>> +    int64_t value;
>> +    qmp_input_type_int(v,&value, name, errp);
>> +    *obj = value;
>> +}
>> +
>> +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);
>> +
>> +    if (!qobj) {
>> +        *present = false;
>> +        return;
>> +    }
>> +
>> +    *present = true;
>> +}
>> +
>> +static void qmp_input_end_optional(Visitor *v, Error **errp)
>> +{
>> +}
>> +
>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
>> +{
>> +    return&v->visitor;
>> +}
>> +
>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
>> +{
>
> We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and
> decrefing here.
>

I think it was either that, or just making the QObject arg to 
qmp_input_visitor_new() const, to make it clear it was caller-freed.

>> +    qemu_free(v);
>> +}
>> +
>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
>> +{
>> +    QmpInputVisitor *v;
>> +
>> +    v = qemu_mallocz(sizeof(*v));
>> +
>> +    v->visitor.start_struct = qmp_input_start_struct;
>> +    v->visitor.end_struct = qmp_input_end_struct;
>> +    v->visitor.start_list = qmp_input_start_list;
>> +    v->visitor.next_list = qmp_input_next_list;
>> +    v->visitor.end_list = qmp_input_end_list;
>> +    v->visitor.type_enum = qmp_input_type_enum;
>> +    v->visitor.type_int = qmp_input_type_int;
>> +    v->visitor.type_bool = qmp_input_type_bool;
>> +    v->visitor.type_str = qmp_input_type_str;
>> +    v->visitor.type_number = qmp_input_type_number;
>> +    v->visitor.start_optional = qmp_input_start_optional;
>> +    v->visitor.end_optional = qmp_input_end_optional;
>> +
>> +    v->obj = obj;
>> +
>> +    return v;
>> +}
>> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
>> new file mode 100644
>> index 0000000..0f77916
>> --- /dev/null
>> +++ b/qapi/qmp-input-visitor.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * Input Visitor
>> + *
>> + * Copyright IBM, Corp. 2011
>> + *
>> + * Authors:
>> + *  Anthony Liguori<aliguori@us.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef QMP_INPUT_VISITOR_H
>> +#define QMP_INPUT_VISITOR_H
>> +
>> +#include "qapi-visit-core.h"
>> +#include "qobject.h"
>> +
>> +typedef struct QmpInputVisitor QmpInputVisitor;
>> +
>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v);
>> +
>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
>> +
>> +#endif
>> diff --git a/qerror.h b/qerror.h
>> index 16c830d..9a9fa5b 100644
>> --- a/qerror.h
>> +++ b/qerror.h
>> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj);
>>   #define QERR_JSON_PARSE_ERROR \
>>       "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
>>
>> +#define QERR_BUFFER_OVERRUN \
>> +    "{ 'class': 'BufferOverrun', 'data': {} }"
>> +
>
> You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory,
> but BufferOverrrun is probably fine.
>
>>   #define QERR_KVM_MISSING_CAP \
>>       "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
>>
>
Luiz Capitulino - July 12, 2011, 1:16 p.m.
On Mon, 11 Jul 2011 19:05:58 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 07/07/2011 09:32 AM, Luiz Capitulino wrote:
> > On Tue,  5 Jul 2011 08:02:32 -0500
> > Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
> >
> >> A type of Visiter class that is used to walk a qobject's
> >> structure and assign each entry to the corresponding native C type.
> >> Command marshaling function will use this to pull out QMP command
> >> parameters recieved over the wire and pass them as native arguments
> >> to the corresponding C functions.
> >>
> >> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
> >> ---
> >>   Makefile.objs            |    2 +-
> >>   qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
> >>   qapi/qmp-input-visitor.h |   27 +++++
> >>   qerror.h                 |    3 +
> >>   4 files changed, 295 insertions(+), 1 deletions(-)
> >>   create mode 100644 qapi/qmp-input-visitor.c
> >>   create mode 100644 qapi/qmp-input-visitor.h
> >>
> >> diff --git a/Makefile.objs b/Makefile.objs
> >> index 0077014..997ecef 100644
> >> --- a/Makefile.objs
> >> +++ b/Makefile.objs
> >> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
> >>   ######################################################################
> >>   # qapi
> >>
> >> -qapi-nested-y = qapi-visit-core.o
> >> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
> >>   qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
> >>
> >>   vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> >> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> >> new file mode 100644
> >> index 0000000..80912bb
> >> --- /dev/null
> >> +++ b/qapi/qmp-input-visitor.c
> >> @@ -0,0 +1,264 @@
> >> +/*
> >> + * Input Visitor
> >> + *
> >> + * Copyright IBM, Corp. 2011
> >> + *
> >> + * Authors:
> >> + *  Anthony Liguori<aliguori@us.ibm.com>
> >> + *
> >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >> + * See the COPYING.LIB file in the top-level directory.
> >> + *
> >> + */
> >> +
> >> +#include "qmp-input-visitor.h"
> >> +#include "qemu-queue.h"
> >> +#include "qemu-common.h"
> >> +#include "qemu-objects.h"
> >> +#include "qerror.h"
> >> +
> >> +#define QIV_STACK_SIZE 1024
> >> +
> >> +typedef struct StackObject
> >> +{
> >> +    const QObject *obj;
> >> +    const  QListEntry *entry;
> >> +} StackObject;
> >> +
> >> +struct QmpInputVisitor
> >> +{
> >> +    Visitor visitor;
> >> +    const QObject *obj;
> >> +    StackObject stack[QIV_STACK_SIZE];
> >> +    int nb_stack;
> >> +};
> >> +
> >> +static QmpInputVisitor *to_qiv(Visitor *v)
> >> +{
> >> +    return container_of(v, QmpInputVisitor, visitor);
> >> +}
> >> +
> >> +static const 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;
> >> +    }
> >> +
> >> +    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) {
> >> +        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
> >> +    }
> >> +
> >> +    return qobj;
> >> +}
> >> +
> >> +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->nb_stack++;
> >> +
> >> +    if (qiv->nb_stack>= QIV_STACK_SIZE) {
> >> +        error_set(errp, QERR_BUFFER_OVERRUN);
> >> +        return;
> >> +    }
> >> +}
> >> +
> >> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
> >> +{
> >> +    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, const char *name, size_t size, Error **errp)
> >> +{
> >> +    QmpInputVisitor *qiv = to_qiv(v);
> >> +    const QObject *qobj = qmp_input_get_object(qiv, name);
> >> +
> >> +    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
> >> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
> >> +        return;
> >> +    }
> >> +
> >> +    qmp_input_push(qiv, qobj, errp);
> >> +    if (error_is_set(errp)) {
> >> +        return;
> >> +    }
> >> +
> >> +    if (obj) {
> >> +        *obj = qemu_mallocz(size);
> >> +    }
> >> +}
> >> +
> >> +static void qmp_input_end_struct(Visitor *v, Error **errp)
> >> +{
> >> +    QmpInputVisitor *qiv = to_qiv(v);
> >> +
> >> +    qmp_input_pop(qiv, 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);
> >> +
> >> +    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
> >> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
> >> +        return;
> >> +    }
> >> +
> >> +    qmp_input_push(qiv, qobj, errp);
> >> +}
> >> +
> >> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
> >> +{
> >> +    QmpInputVisitor *qiv = to_qiv(v);
> >> +    GenericList *entry;
> >> +    StackObject *so =&qiv->stack[qiv->nb_stack - 1];
> >> +
> >> +    if (so->entry == NULL) {
> >> +        return NULL;
> >> +    }
> >> +
> >> +    entry = qemu_mallocz(sizeof(*entry));
> >> +    if (*list) {
> >> +        so->entry = qlist_next(so->entry);
> >> +        if (so->entry == NULL) {
> >> +            qemu_free(entry);
> >> +            return NULL;
> >> +        }
> >> +        (*list)->next = entry;
> >> +    }
> >> +    *list = entry;
> >> +
> >> +
> >> +    return entry;
> >> +}
> >> +
> >> +static void qmp_input_end_list(Visitor *v, Error **errp)
> >> +{
> >> +    QmpInputVisitor *qiv = to_qiv(v);
> >> +
> >> +    qmp_input_pop(qiv, errp);
> >> +}
> >> +
> >> +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);
> >> +
> >> +    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
> >> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
> >> +        return;
> >> +    }
> >> +
> >> +    *obj = qint_get_int(qobject_to_qint(qobj));
> >> +}
> >> +
> >> +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);
> >> +
> >> +    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
> >> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
> >> +        return;
> >> +    }
> >> +
> >> +    *obj = qbool_get_int(qobject_to_qbool(qobj));
> >> +}
> >> +
> >> +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);
> >> +
> >> +    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
> >> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
> >> +        return;
> >> +    }
> >> +
> >> +    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
> >> +}
> >> +
> >> +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);
> >> +
> >> +    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
> >> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
> >> +        return;
> >> +    }
> >> +
> >> +    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
> >> +}
> >> +
> >> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
> >> +{
> >> +    int64_t value;
> >> +    qmp_input_type_int(v,&value, name, errp);
> >> +    *obj = value;
> >> +}
> >> +
> >> +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);
> >> +
> >> +    if (!qobj) {
> >> +        *present = false;
> >> +        return;
> >> +    }
> >> +
> >> +    *present = true;
> >> +}
> >> +
> >> +static void qmp_input_end_optional(Visitor *v, Error **errp)
> >> +{
> >> +}
> >> +
> >> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
> >> +{
> >> +    return&v->visitor;
> >> +}
> >> +
> >> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
> >> +{
> >
> > We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and
> > decrefing here.
> >
> 
> I think it was either that, or just making the QObject arg to 
> qmp_input_visitor_new() const, to make it clear it was caller-freed.

No, because (at least theoretically) a client could free it before walking
the struct with the visitor functions. The rule is: if you store it, you
have to increment its reference or get its ownership.


> 
> >> +    qemu_free(v);
> >> +}
> >> +
> >> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
> >> +{
> >> +    QmpInputVisitor *v;
> >> +
> >> +    v = qemu_mallocz(sizeof(*v));
> >> +
> >> +    v->visitor.start_struct = qmp_input_start_struct;
> >> +    v->visitor.end_struct = qmp_input_end_struct;
> >> +    v->visitor.start_list = qmp_input_start_list;
> >> +    v->visitor.next_list = qmp_input_next_list;
> >> +    v->visitor.end_list = qmp_input_end_list;
> >> +    v->visitor.type_enum = qmp_input_type_enum;
> >> +    v->visitor.type_int = qmp_input_type_int;
> >> +    v->visitor.type_bool = qmp_input_type_bool;
> >> +    v->visitor.type_str = qmp_input_type_str;
> >> +    v->visitor.type_number = qmp_input_type_number;
> >> +    v->visitor.start_optional = qmp_input_start_optional;
> >> +    v->visitor.end_optional = qmp_input_end_optional;
> >> +
> >> +    v->obj = obj;
> >> +
> >> +    return v;
> >> +}
> >> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
> >> new file mode 100644
> >> index 0000000..0f77916
> >> --- /dev/null
> >> +++ b/qapi/qmp-input-visitor.h
> >> @@ -0,0 +1,27 @@
> >> +/*
> >> + * Input Visitor
> >> + *
> >> + * Copyright IBM, Corp. 2011
> >> + *
> >> + * Authors:
> >> + *  Anthony Liguori<aliguori@us.ibm.com>
> >> + *
> >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >> + * See the COPYING.LIB file in the top-level directory.
> >> + *
> >> + */
> >> +
> >> +#ifndef QMP_INPUT_VISITOR_H
> >> +#define QMP_INPUT_VISITOR_H
> >> +
> >> +#include "qapi-visit-core.h"
> >> +#include "qobject.h"
> >> +
> >> +typedef struct QmpInputVisitor QmpInputVisitor;
> >> +
> >> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
> >> +void qmp_input_visitor_cleanup(QmpInputVisitor *v);
> >> +
> >> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
> >> +
> >> +#endif
> >> diff --git a/qerror.h b/qerror.h
> >> index 16c830d..9a9fa5b 100644
> >> --- a/qerror.h
> >> +++ b/qerror.h
> >> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj);
> >>   #define QERR_JSON_PARSE_ERROR \
> >>       "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
> >>
> >> +#define QERR_BUFFER_OVERRUN \
> >> +    "{ 'class': 'BufferOverrun', 'data': {} }"
> >> +
> >
> > You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory,
> > but BufferOverrrun is probably fine.
> >
> >>   #define QERR_KVM_MISSING_CAP \
> >>       "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
> >>
> >
>
Michael Roth - July 12, 2011, 1:46 p.m.
On 07/12/2011 08:16 AM, Luiz Capitulino wrote:
> On Mon, 11 Jul 2011 19:05:58 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> On 07/07/2011 09:32 AM, Luiz Capitulino wrote:
>>> On Tue,  5 Jul 2011 08:02:32 -0500
>>> Michael Roth<mdroth@linux.vnet.ibm.com>   wrote:
>>>
>>>> A type of Visiter class that is used to walk a qobject's
>>>> structure and assign each entry to the corresponding native C type.
>>>> Command marshaling function will use this to pull out QMP command
>>>> parameters recieved over the wire and pass them as native arguments
>>>> to the corresponding C functions.
>>>>
>>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>>> ---
>>>>    Makefile.objs            |    2 +-
>>>>    qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>    qapi/qmp-input-visitor.h |   27 +++++
>>>>    qerror.h                 |    3 +
>>>>    4 files changed, 295 insertions(+), 1 deletions(-)
>>>>    create mode 100644 qapi/qmp-input-visitor.c
>>>>    create mode 100644 qapi/qmp-input-visitor.h
>>>>
>>>> diff --git a/Makefile.objs b/Makefile.objs
>>>> index 0077014..997ecef 100644
>>>> --- a/Makefile.objs
>>>> +++ b/Makefile.objs
>>>> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
>>>>    ######################################################################
>>>>    # qapi
>>>>
>>>> -qapi-nested-y = qapi-visit-core.o
>>>> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
>>>>    qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>>>>
>>>>    vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>>>> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
>>>> new file mode 100644
>>>> index 0000000..80912bb
>>>> --- /dev/null
>>>> +++ b/qapi/qmp-input-visitor.c
>>>> @@ -0,0 +1,264 @@
>>>> +/*
>>>> + * Input Visitor
>>>> + *
>>>> + * Copyright IBM, Corp. 2011
>>>> + *
>>>> + * Authors:
>>>> + *  Anthony Liguori<aliguori@us.ibm.com>
>>>> + *
>>>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>>>> + * See the COPYING.LIB file in the top-level directory.
>>>> + *
>>>> + */
>>>> +
>>>> +#include "qmp-input-visitor.h"
>>>> +#include "qemu-queue.h"
>>>> +#include "qemu-common.h"
>>>> +#include "qemu-objects.h"
>>>> +#include "qerror.h"
>>>> +
>>>> +#define QIV_STACK_SIZE 1024
>>>> +
>>>> +typedef struct StackObject
>>>> +{
>>>> +    const QObject *obj;
>>>> +    const  QListEntry *entry;
>>>> +} StackObject;
>>>> +
>>>> +struct QmpInputVisitor
>>>> +{
>>>> +    Visitor visitor;
>>>> +    const QObject *obj;
>>>> +    StackObject stack[QIV_STACK_SIZE];
>>>> +    int nb_stack;
>>>> +};
>>>> +
>>>> +static QmpInputVisitor *to_qiv(Visitor *v)
>>>> +{
>>>> +    return container_of(v, QmpInputVisitor, visitor);
>>>> +}
>>>> +
>>>> +static const 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;
>>>> +    }
>>>> +
>>>> +    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) {
>>>> +        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
>>>> +    }
>>>> +
>>>> +    return qobj;
>>>> +}
>>>> +
>>>> +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->nb_stack++;
>>>> +
>>>> +    if (qiv->nb_stack>= QIV_STACK_SIZE) {
>>>> +        error_set(errp, QERR_BUFFER_OVERRUN);
>>>> +        return;
>>>> +    }
>>>> +}
>>>> +
>>>> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
>>>> +{
>>>> +    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, const char *name, size_t size, Error **errp)
>>>> +{
>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>> +    const QObject *qobj = qmp_input_get_object(qiv, name);
>>>> +
>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    qmp_input_push(qiv, qobj, errp);
>>>> +    if (error_is_set(errp)) {
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (obj) {
>>>> +        *obj = qemu_mallocz(size);
>>>> +    }
>>>> +}
>>>> +
>>>> +static void qmp_input_end_struct(Visitor *v, Error **errp)
>>>> +{
>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>> +
>>>> +    qmp_input_pop(qiv, 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);
>>>> +
>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    qmp_input_push(qiv, qobj, errp);
>>>> +}
>>>> +
>>>> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
>>>> +{
>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>> +    GenericList *entry;
>>>> +    StackObject *so =&qiv->stack[qiv->nb_stack - 1];
>>>> +
>>>> +    if (so->entry == NULL) {
>>>> +        return NULL;
>>>> +    }
>>>> +
>>>> +    entry = qemu_mallocz(sizeof(*entry));
>>>> +    if (*list) {
>>>> +        so->entry = qlist_next(so->entry);
>>>> +        if (so->entry == NULL) {
>>>> +            qemu_free(entry);
>>>> +            return NULL;
>>>> +        }
>>>> +        (*list)->next = entry;
>>>> +    }
>>>> +    *list = entry;
>>>> +
>>>> +
>>>> +    return entry;
>>>> +}
>>>> +
>>>> +static void qmp_input_end_list(Visitor *v, Error **errp)
>>>> +{
>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>> +
>>>> +    qmp_input_pop(qiv, errp);
>>>> +}
>>>> +
>>>> +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);
>>>> +
>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    *obj = qint_get_int(qobject_to_qint(qobj));
>>>> +}
>>>> +
>>>> +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);
>>>> +
>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    *obj = qbool_get_int(qobject_to_qbool(qobj));
>>>> +}
>>>> +
>>>> +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);
>>>> +
>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
>>>> +}
>>>> +
>>>> +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);
>>>> +
>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
>>>> +}
>>>> +
>>>> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
>>>> +{
>>>> +    int64_t value;
>>>> +    qmp_input_type_int(v,&value, name, errp);
>>>> +    *obj = value;
>>>> +}
>>>> +
>>>> +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);
>>>> +
>>>> +    if (!qobj) {
>>>> +        *present = false;
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    *present = true;
>>>> +}
>>>> +
>>>> +static void qmp_input_end_optional(Visitor *v, Error **errp)
>>>> +{
>>>> +}
>>>> +
>>>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
>>>> +{
>>>> +    return&v->visitor;
>>>> +}
>>>> +
>>>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
>>>> +{
>>>
>>> We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and
>>> decrefing here.
>>>
>>
>> I think it was either that, or just making the QObject arg to
>> qmp_input_visitor_new() const, to make it clear it was caller-freed.
>
> No, because (at least theoretically) a client could free it before walking
> the struct with the visitor functions. The rule is: if you store it, you
> have to increment its reference or get its ownership.
>
>

Makes sense. Sucks to have to remove the const for that though...would a 
temporary cast to non-const be acceptable in a case like this?

>>
>>>> +    qemu_free(v);
>>>> +}
>>>> +
>>>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
>>>> +{
>>>> +    QmpInputVisitor *v;
>>>> +
>>>> +    v = qemu_mallocz(sizeof(*v));
>>>> +
>>>> +    v->visitor.start_struct = qmp_input_start_struct;
>>>> +    v->visitor.end_struct = qmp_input_end_struct;
>>>> +    v->visitor.start_list = qmp_input_start_list;
>>>> +    v->visitor.next_list = qmp_input_next_list;
>>>> +    v->visitor.end_list = qmp_input_end_list;
>>>> +    v->visitor.type_enum = qmp_input_type_enum;
>>>> +    v->visitor.type_int = qmp_input_type_int;
>>>> +    v->visitor.type_bool = qmp_input_type_bool;
>>>> +    v->visitor.type_str = qmp_input_type_str;
>>>> +    v->visitor.type_number = qmp_input_type_number;
>>>> +    v->visitor.start_optional = qmp_input_start_optional;
>>>> +    v->visitor.end_optional = qmp_input_end_optional;
>>>> +
>>>> +    v->obj = obj;
>>>> +
>>>> +    return v;
>>>> +}
>>>> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
>>>> new file mode 100644
>>>> index 0000000..0f77916
>>>> --- /dev/null
>>>> +++ b/qapi/qmp-input-visitor.h
>>>> @@ -0,0 +1,27 @@
>>>> +/*
>>>> + * Input Visitor
>>>> + *
>>>> + * Copyright IBM, Corp. 2011
>>>> + *
>>>> + * Authors:
>>>> + *  Anthony Liguori<aliguori@us.ibm.com>
>>>> + *
>>>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>>>> + * See the COPYING.LIB file in the top-level directory.
>>>> + *
>>>> + */
>>>> +
>>>> +#ifndef QMP_INPUT_VISITOR_H
>>>> +#define QMP_INPUT_VISITOR_H
>>>> +
>>>> +#include "qapi-visit-core.h"
>>>> +#include "qobject.h"
>>>> +
>>>> +typedef struct QmpInputVisitor QmpInputVisitor;
>>>> +
>>>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
>>>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v);
>>>> +
>>>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
>>>> +
>>>> +#endif
>>>> diff --git a/qerror.h b/qerror.h
>>>> index 16c830d..9a9fa5b 100644
>>>> --- a/qerror.h
>>>> +++ b/qerror.h
>>>> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj);
>>>>    #define QERR_JSON_PARSE_ERROR \
>>>>        "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
>>>>
>>>> +#define QERR_BUFFER_OVERRUN \
>>>> +    "{ 'class': 'BufferOverrun', 'data': {} }"
>>>> +
>>>
>>> You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory,
>>> but BufferOverrrun is probably fine.
>>>
>>>>    #define QERR_KVM_MISSING_CAP \
>>>>        "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
>>>>
>>>
>>
>
Luiz Capitulino - July 12, 2011, 1:53 p.m.
On Tue, 12 Jul 2011 08:46:13 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 07/12/2011 08:16 AM, Luiz Capitulino wrote:
> > On Mon, 11 Jul 2011 19:05:58 -0500
> > Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
> >
> >> On 07/07/2011 09:32 AM, Luiz Capitulino wrote:
> >>> On Tue,  5 Jul 2011 08:02:32 -0500
> >>> Michael Roth<mdroth@linux.vnet.ibm.com>   wrote:
> >>>
> >>>> A type of Visiter class that is used to walk a qobject's
> >>>> structure and assign each entry to the corresponding native C type.
> >>>> Command marshaling function will use this to pull out QMP command
> >>>> parameters recieved over the wire and pass them as native arguments
> >>>> to the corresponding C functions.
> >>>>
> >>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
> >>>> ---
> >>>>    Makefile.objs            |    2 +-
> >>>>    qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
> >>>>    qapi/qmp-input-visitor.h |   27 +++++
> >>>>    qerror.h                 |    3 +
> >>>>    4 files changed, 295 insertions(+), 1 deletions(-)
> >>>>    create mode 100644 qapi/qmp-input-visitor.c
> >>>>    create mode 100644 qapi/qmp-input-visitor.h
> >>>>
> >>>> diff --git a/Makefile.objs b/Makefile.objs
> >>>> index 0077014..997ecef 100644
> >>>> --- a/Makefile.objs
> >>>> +++ b/Makefile.objs
> >>>> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
> >>>>    ######################################################################
> >>>>    # qapi
> >>>>
> >>>> -qapi-nested-y = qapi-visit-core.o
> >>>> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
> >>>>    qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
> >>>>
> >>>>    vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
> >>>> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> >>>> new file mode 100644
> >>>> index 0000000..80912bb
> >>>> --- /dev/null
> >>>> +++ b/qapi/qmp-input-visitor.c
> >>>> @@ -0,0 +1,264 @@
> >>>> +/*
> >>>> + * Input Visitor
> >>>> + *
> >>>> + * Copyright IBM, Corp. 2011
> >>>> + *
> >>>> + * Authors:
> >>>> + *  Anthony Liguori<aliguori@us.ibm.com>
> >>>> + *
> >>>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >>>> + * See the COPYING.LIB file in the top-level directory.
> >>>> + *
> >>>> + */
> >>>> +
> >>>> +#include "qmp-input-visitor.h"
> >>>> +#include "qemu-queue.h"
> >>>> +#include "qemu-common.h"
> >>>> +#include "qemu-objects.h"
> >>>> +#include "qerror.h"
> >>>> +
> >>>> +#define QIV_STACK_SIZE 1024
> >>>> +
> >>>> +typedef struct StackObject
> >>>> +{
> >>>> +    const QObject *obj;
> >>>> +    const  QListEntry *entry;
> >>>> +} StackObject;
> >>>> +
> >>>> +struct QmpInputVisitor
> >>>> +{
> >>>> +    Visitor visitor;
> >>>> +    const QObject *obj;
> >>>> +    StackObject stack[QIV_STACK_SIZE];
> >>>> +    int nb_stack;
> >>>> +};
> >>>> +
> >>>> +static QmpInputVisitor *to_qiv(Visitor *v)
> >>>> +{
> >>>> +    return container_of(v, QmpInputVisitor, visitor);
> >>>> +}
> >>>> +
> >>>> +static const 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;
> >>>> +    }
> >>>> +
> >>>> +    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) {
> >>>> +        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
> >>>> +    }
> >>>> +
> >>>> +    return qobj;
> >>>> +}
> >>>> +
> >>>> +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->nb_stack++;
> >>>> +
> >>>> +    if (qiv->nb_stack>= QIV_STACK_SIZE) {
> >>>> +        error_set(errp, QERR_BUFFER_OVERRUN);
> >>>> +        return;
> >>>> +    }
> >>>> +}
> >>>> +
> >>>> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
> >>>> +{
> >>>> +    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, const char *name, size_t size, Error **errp)
> >>>> +{
> >>>> +    QmpInputVisitor *qiv = to_qiv(v);
> >>>> +    const QObject *qobj = qmp_input_get_object(qiv, name);
> >>>> +
> >>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
> >>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    qmp_input_push(qiv, qobj, errp);
> >>>> +    if (error_is_set(errp)) {
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    if (obj) {
> >>>> +        *obj = qemu_mallocz(size);
> >>>> +    }
> >>>> +}
> >>>> +
> >>>> +static void qmp_input_end_struct(Visitor *v, Error **errp)
> >>>> +{
> >>>> +    QmpInputVisitor *qiv = to_qiv(v);
> >>>> +
> >>>> +    qmp_input_pop(qiv, 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);
> >>>> +
> >>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
> >>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    qmp_input_push(qiv, qobj, errp);
> >>>> +}
> >>>> +
> >>>> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
> >>>> +{
> >>>> +    QmpInputVisitor *qiv = to_qiv(v);
> >>>> +    GenericList *entry;
> >>>> +    StackObject *so =&qiv->stack[qiv->nb_stack - 1];
> >>>> +
> >>>> +    if (so->entry == NULL) {
> >>>> +        return NULL;
> >>>> +    }
> >>>> +
> >>>> +    entry = qemu_mallocz(sizeof(*entry));
> >>>> +    if (*list) {
> >>>> +        so->entry = qlist_next(so->entry);
> >>>> +        if (so->entry == NULL) {
> >>>> +            qemu_free(entry);
> >>>> +            return NULL;
> >>>> +        }
> >>>> +        (*list)->next = entry;
> >>>> +    }
> >>>> +    *list = entry;
> >>>> +
> >>>> +
> >>>> +    return entry;
> >>>> +}
> >>>> +
> >>>> +static void qmp_input_end_list(Visitor *v, Error **errp)
> >>>> +{
> >>>> +    QmpInputVisitor *qiv = to_qiv(v);
> >>>> +
> >>>> +    qmp_input_pop(qiv, errp);
> >>>> +}
> >>>> +
> >>>> +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);
> >>>> +
> >>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
> >>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    *obj = qint_get_int(qobject_to_qint(qobj));
> >>>> +}
> >>>> +
> >>>> +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);
> >>>> +
> >>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
> >>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    *obj = qbool_get_int(qobject_to_qbool(qobj));
> >>>> +}
> >>>> +
> >>>> +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);
> >>>> +
> >>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
> >>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
> >>>> +}
> >>>> +
> >>>> +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);
> >>>> +
> >>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
> >>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
> >>>> +}
> >>>> +
> >>>> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
> >>>> +{
> >>>> +    int64_t value;
> >>>> +    qmp_input_type_int(v,&value, name, errp);
> >>>> +    *obj = value;
> >>>> +}
> >>>> +
> >>>> +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);
> >>>> +
> >>>> +    if (!qobj) {
> >>>> +        *present = false;
> >>>> +        return;
> >>>> +    }
> >>>> +
> >>>> +    *present = true;
> >>>> +}
> >>>> +
> >>>> +static void qmp_input_end_optional(Visitor *v, Error **errp)
> >>>> +{
> >>>> +}
> >>>> +
> >>>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
> >>>> +{
> >>>> +    return&v->visitor;
> >>>> +}
> >>>> +
> >>>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
> >>>> +{
> >>>
> >>> We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and
> >>> decrefing here.
> >>>
> >>
> >> I think it was either that, or just making the QObject arg to
> >> qmp_input_visitor_new() const, to make it clear it was caller-freed.
> >
> > No, because (at least theoretically) a client could free it before walking
> > the struct with the visitor functions. The rule is: if you store it, you
> > have to increment its reference or get its ownership.
> >
> >
> 
> Makes sense. Sucks to have to remove the const for that though...would a 
> temporary cast to non-const be acceptable in a case like this?

Well, if we have to change it, it's not const anymore :)

> 
> >>
> >>>> +    qemu_free(v);
> >>>> +}
> >>>> +
> >>>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
> >>>> +{
> >>>> +    QmpInputVisitor *v;
> >>>> +
> >>>> +    v = qemu_mallocz(sizeof(*v));
> >>>> +
> >>>> +    v->visitor.start_struct = qmp_input_start_struct;
> >>>> +    v->visitor.end_struct = qmp_input_end_struct;
> >>>> +    v->visitor.start_list = qmp_input_start_list;
> >>>> +    v->visitor.next_list = qmp_input_next_list;
> >>>> +    v->visitor.end_list = qmp_input_end_list;
> >>>> +    v->visitor.type_enum = qmp_input_type_enum;
> >>>> +    v->visitor.type_int = qmp_input_type_int;
> >>>> +    v->visitor.type_bool = qmp_input_type_bool;
> >>>> +    v->visitor.type_str = qmp_input_type_str;
> >>>> +    v->visitor.type_number = qmp_input_type_number;
> >>>> +    v->visitor.start_optional = qmp_input_start_optional;
> >>>> +    v->visitor.end_optional = qmp_input_end_optional;
> >>>> +
> >>>> +    v->obj = obj;
> >>>> +
> >>>> +    return v;
> >>>> +}
> >>>> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
> >>>> new file mode 100644
> >>>> index 0000000..0f77916
> >>>> --- /dev/null
> >>>> +++ b/qapi/qmp-input-visitor.h
> >>>> @@ -0,0 +1,27 @@
> >>>> +/*
> >>>> + * Input Visitor
> >>>> + *
> >>>> + * Copyright IBM, Corp. 2011
> >>>> + *
> >>>> + * Authors:
> >>>> + *  Anthony Liguori<aliguori@us.ibm.com>
> >>>> + *
> >>>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> >>>> + * See the COPYING.LIB file in the top-level directory.
> >>>> + *
> >>>> + */
> >>>> +
> >>>> +#ifndef QMP_INPUT_VISITOR_H
> >>>> +#define QMP_INPUT_VISITOR_H
> >>>> +
> >>>> +#include "qapi-visit-core.h"
> >>>> +#include "qobject.h"
> >>>> +
> >>>> +typedef struct QmpInputVisitor QmpInputVisitor;
> >>>> +
> >>>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
> >>>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v);
> >>>> +
> >>>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
> >>>> +
> >>>> +#endif
> >>>> diff --git a/qerror.h b/qerror.h
> >>>> index 16c830d..9a9fa5b 100644
> >>>> --- a/qerror.h
> >>>> +++ b/qerror.h
> >>>> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj);
> >>>>    #define QERR_JSON_PARSE_ERROR \
> >>>>        "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
> >>>>
> >>>> +#define QERR_BUFFER_OVERRUN \
> >>>> +    "{ 'class': 'BufferOverrun', 'data': {} }"
> >>>> +
> >>>
> >>> You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory,
> >>> but BufferOverrrun is probably fine.
> >>>
> >>>>    #define QERR_KVM_MISSING_CAP \
> >>>>        "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
> >>>>
> >>>
> >>
> >
>
Michael Roth - July 12, 2011, 2:14 p.m.
On 07/12/2011 08:53 AM, Luiz Capitulino wrote:
> On Tue, 12 Jul 2011 08:46:13 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> On 07/12/2011 08:16 AM, Luiz Capitulino wrote:
>>> On Mon, 11 Jul 2011 19:05:58 -0500
>>> Michael Roth<mdroth@linux.vnet.ibm.com>   wrote:
>>>
>>>> On 07/07/2011 09:32 AM, Luiz Capitulino wrote:
>>>>> On Tue,  5 Jul 2011 08:02:32 -0500
>>>>> Michael Roth<mdroth@linux.vnet.ibm.com>    wrote:
>>>>>
>>>>>> A type of Visiter class that is used to walk a qobject's
>>>>>> structure and assign each entry to the corresponding native C type.
>>>>>> Command marshaling function will use this to pull out QMP command
>>>>>> parameters recieved over the wire and pass them as native arguments
>>>>>> to the corresponding C functions.
>>>>>>
>>>>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>>>>> ---
>>>>>>     Makefile.objs            |    2 +-
>>>>>>     qapi/qmp-input-visitor.c |  264 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>     qapi/qmp-input-visitor.h |   27 +++++
>>>>>>     qerror.h                 |    3 +
>>>>>>     4 files changed, 295 insertions(+), 1 deletions(-)
>>>>>>     create mode 100644 qapi/qmp-input-visitor.c
>>>>>>     create mode 100644 qapi/qmp-input-visitor.h
>>>>>>
>>>>>> diff --git a/Makefile.objs b/Makefile.objs
>>>>>> index 0077014..997ecef 100644
>>>>>> --- a/Makefile.objs
>>>>>> +++ b/Makefile.objs
>>>>>> @@ -375,7 +375,7 @@ libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
>>>>>>     ######################################################################
>>>>>>     # qapi
>>>>>>
>>>>>> -qapi-nested-y = qapi-visit-core.o
>>>>>> +qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
>>>>>>     qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
>>>>>>
>>>>>>     vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>>>>>> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
>>>>>> new file mode 100644
>>>>>> index 0000000..80912bb
>>>>>> --- /dev/null
>>>>>> +++ b/qapi/qmp-input-visitor.c
>>>>>> @@ -0,0 +1,264 @@
>>>>>> +/*
>>>>>> + * Input Visitor
>>>>>> + *
>>>>>> + * Copyright IBM, Corp. 2011
>>>>>> + *
>>>>>> + * Authors:
>>>>>> + *  Anthony Liguori<aliguori@us.ibm.com>
>>>>>> + *
>>>>>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>>>>>> + * See the COPYING.LIB file in the top-level directory.
>>>>>> + *
>>>>>> + */
>>>>>> +
>>>>>> +#include "qmp-input-visitor.h"
>>>>>> +#include "qemu-queue.h"
>>>>>> +#include "qemu-common.h"
>>>>>> +#include "qemu-objects.h"
>>>>>> +#include "qerror.h"
>>>>>> +
>>>>>> +#define QIV_STACK_SIZE 1024
>>>>>> +
>>>>>> +typedef struct StackObject
>>>>>> +{
>>>>>> +    const QObject *obj;
>>>>>> +    const  QListEntry *entry;
>>>>>> +} StackObject;
>>>>>> +
>>>>>> +struct QmpInputVisitor
>>>>>> +{
>>>>>> +    Visitor visitor;
>>>>>> +    const QObject *obj;
>>>>>> +    StackObject stack[QIV_STACK_SIZE];
>>>>>> +    int nb_stack;
>>>>>> +};
>>>>>> +
>>>>>> +static QmpInputVisitor *to_qiv(Visitor *v)
>>>>>> +{
>>>>>> +    return container_of(v, QmpInputVisitor, visitor);
>>>>>> +}
>>>>>> +
>>>>>> +static const 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;
>>>>>> +    }
>>>>>> +
>>>>>> +    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) {
>>>>>> +        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
>>>>>> +    }
>>>>>> +
>>>>>> +    return qobj;
>>>>>> +}
>>>>>> +
>>>>>> +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->nb_stack++;
>>>>>> +
>>>>>> +    if (qiv->nb_stack>= QIV_STACK_SIZE) {
>>>>>> +        error_set(errp, QERR_BUFFER_OVERRUN);
>>>>>> +        return;
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
>>>>>> +{
>>>>>> +    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, const char *name, size_t size, Error **errp)
>>>>>> +{
>>>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>>>> +    const QObject *qobj = qmp_input_get_object(qiv, name);
>>>>>> +
>>>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
>>>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    qmp_input_push(qiv, qobj, errp);
>>>>>> +    if (error_is_set(errp)) {
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    if (obj) {
>>>>>> +        *obj = qemu_mallocz(size);
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>> +static void qmp_input_end_struct(Visitor *v, Error **errp)
>>>>>> +{
>>>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>>>> +
>>>>>> +    qmp_input_pop(qiv, 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);
>>>>>> +
>>>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
>>>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    qmp_input_push(qiv, qobj, errp);
>>>>>> +}
>>>>>> +
>>>>>> +static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
>>>>>> +{
>>>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>>>> +    GenericList *entry;
>>>>>> +    StackObject *so =&qiv->stack[qiv->nb_stack - 1];
>>>>>> +
>>>>>> +    if (so->entry == NULL) {
>>>>>> +        return NULL;
>>>>>> +    }
>>>>>> +
>>>>>> +    entry = qemu_mallocz(sizeof(*entry));
>>>>>> +    if (*list) {
>>>>>> +        so->entry = qlist_next(so->entry);
>>>>>> +        if (so->entry == NULL) {
>>>>>> +            qemu_free(entry);
>>>>>> +            return NULL;
>>>>>> +        }
>>>>>> +        (*list)->next = entry;
>>>>>> +    }
>>>>>> +    *list = entry;
>>>>>> +
>>>>>> +
>>>>>> +    return entry;
>>>>>> +}
>>>>>> +
>>>>>> +static void qmp_input_end_list(Visitor *v, Error **errp)
>>>>>> +{
>>>>>> +    QmpInputVisitor *qiv = to_qiv(v);
>>>>>> +
>>>>>> +    qmp_input_pop(qiv, errp);
>>>>>> +}
>>>>>> +
>>>>>> +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);
>>>>>> +
>>>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
>>>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    *obj = qint_get_int(qobject_to_qint(qobj));
>>>>>> +}
>>>>>> +
>>>>>> +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);
>>>>>> +
>>>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
>>>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    *obj = qbool_get_int(qobject_to_qbool(qobj));
>>>>>> +}
>>>>>> +
>>>>>> +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);
>>>>>> +
>>>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
>>>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
>>>>>> +}
>>>>>> +
>>>>>> +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);
>>>>>> +
>>>>>> +    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
>>>>>> +        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
>>>>>> +}
>>>>>> +
>>>>>> +static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
>>>>>> +{
>>>>>> +    int64_t value;
>>>>>> +    qmp_input_type_int(v,&value, name, errp);
>>>>>> +    *obj = value;
>>>>>> +}
>>>>>> +
>>>>>> +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);
>>>>>> +
>>>>>> +    if (!qobj) {
>>>>>> +        *present = false;
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>>> +    *present = true;
>>>>>> +}
>>>>>> +
>>>>>> +static void qmp_input_end_optional(Visitor *v, Error **errp)
>>>>>> +{
>>>>>> +}
>>>>>> +
>>>>>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
>>>>>> +{
>>>>>> +    return&v->visitor;
>>>>>> +}
>>>>>> +
>>>>>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
>>>>>> +{
>>>>>
>>>>> We talked about doing a qobject_incref(v->obj) in qmp_input_visitor_new() and
>>>>> decrefing here.
>>>>>
>>>>
>>>> I think it was either that, or just making the QObject arg to
>>>> qmp_input_visitor_new() const, to make it clear it was caller-freed.
>>>
>>> No, because (at least theoretically) a client could free it before walking
>>> the struct with the visitor functions. The rule is: if you store it, you
>>> have to increment its reference or get its ownership.
>>>
>>>
>>
>> Makes sense. Sucks to have to remove the const for that though...would a
>> temporary cast to non-const be acceptable in a case like this?
>
> Well, if we have to change it, it's not const anymore :)
>

Heh, yah, silly question. Though from a documentation/api perspective, 
const also helps in terms of making it clear that the dict entries are 
read-only... but not to the point of violating the semantics of const i 
suppose. Only other thing we could do is store a copy instead, but 
that's probably unnecessarily costly, particularly with guest-file-write

>>
>>>>
>>>>>> +    qemu_free(v);
>>>>>> +}
>>>>>> +
>>>>>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
>>>>>> +{
>>>>>> +    QmpInputVisitor *v;
>>>>>> +
>>>>>> +    v = qemu_mallocz(sizeof(*v));
>>>>>> +
>>>>>> +    v->visitor.start_struct = qmp_input_start_struct;
>>>>>> +    v->visitor.end_struct = qmp_input_end_struct;
>>>>>> +    v->visitor.start_list = qmp_input_start_list;
>>>>>> +    v->visitor.next_list = qmp_input_next_list;
>>>>>> +    v->visitor.end_list = qmp_input_end_list;
>>>>>> +    v->visitor.type_enum = qmp_input_type_enum;
>>>>>> +    v->visitor.type_int = qmp_input_type_int;
>>>>>> +    v->visitor.type_bool = qmp_input_type_bool;
>>>>>> +    v->visitor.type_str = qmp_input_type_str;
>>>>>> +    v->visitor.type_number = qmp_input_type_number;
>>>>>> +    v->visitor.start_optional = qmp_input_start_optional;
>>>>>> +    v->visitor.end_optional = qmp_input_end_optional;
>>>>>> +
>>>>>> +    v->obj = obj;
>>>>>> +
>>>>>> +    return v;
>>>>>> +}
>>>>>> diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
>>>>>> new file mode 100644
>>>>>> index 0000000..0f77916
>>>>>> --- /dev/null
>>>>>> +++ b/qapi/qmp-input-visitor.h
>>>>>> @@ -0,0 +1,27 @@
>>>>>> +/*
>>>>>> + * Input Visitor
>>>>>> + *
>>>>>> + * Copyright IBM, Corp. 2011
>>>>>> + *
>>>>>> + * Authors:
>>>>>> + *  Anthony Liguori<aliguori@us.ibm.com>
>>>>>> + *
>>>>>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>>>>>> + * See the COPYING.LIB file in the top-level directory.
>>>>>> + *
>>>>>> + */
>>>>>> +
>>>>>> +#ifndef QMP_INPUT_VISITOR_H
>>>>>> +#define QMP_INPUT_VISITOR_H
>>>>>> +
>>>>>> +#include "qapi-visit-core.h"
>>>>>> +#include "qobject.h"
>>>>>> +
>>>>>> +typedef struct QmpInputVisitor QmpInputVisitor;
>>>>>> +
>>>>>> +QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
>>>>>> +void qmp_input_visitor_cleanup(QmpInputVisitor *v);
>>>>>> +
>>>>>> +Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
>>>>>> +
>>>>>> +#endif
>>>>>> diff --git a/qerror.h b/qerror.h
>>>>>> index 16c830d..9a9fa5b 100644
>>>>>> --- a/qerror.h
>>>>>> +++ b/qerror.h
>>>>>> @@ -124,6 +124,9 @@ QError *qobject_to_qerror(const QObject *obj);
>>>>>>     #define QERR_JSON_PARSE_ERROR \
>>>>>>         "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
>>>>>>
>>>>>> +#define QERR_BUFFER_OVERRUN \
>>>>>> +    "{ 'class': 'BufferOverrun', 'data': {} }"
>>>>>> +
>>>>>
>>>>> You also have to add an entry in qerror.c. I slightly prefer NotEnoughMemory,
>>>>> but BufferOverrrun is probably fine.
>>>>>
>>>>>>     #define QERR_KVM_MISSING_CAP \
>>>>>>         "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"
>>>>>>
>>>>>
>>>>
>>>
>>
>

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 0077014..997ecef 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -375,7 +375,7 @@  libcacard-y = cac.o event.o vcard.o vreader.o vcard_emul_nss.o vcard_emul_type.o
 ######################################################################
 # qapi
 
-qapi-nested-y = qapi-visit-core.o
+qapi-nested-y = qapi-visit-core.o qmp-input-visitor.o
 qapi-obj-y = $(addprefix qapi/, $(qapi-nested-y))
 
 vl.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
new file mode 100644
index 0000000..80912bb
--- /dev/null
+++ b/qapi/qmp-input-visitor.c
@@ -0,0 +1,264 @@ 
+/*
+ * Input Visitor
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qmp-input-visitor.h"
+#include "qemu-queue.h"
+#include "qemu-common.h"
+#include "qemu-objects.h"
+#include "qerror.h"
+
+#define QIV_STACK_SIZE 1024
+
+typedef struct StackObject
+{
+    const QObject *obj;
+    const  QListEntry *entry;
+} StackObject;
+
+struct QmpInputVisitor
+{
+    Visitor visitor;
+    const QObject *obj;
+    StackObject stack[QIV_STACK_SIZE];
+    int nb_stack;
+};
+
+static QmpInputVisitor *to_qiv(Visitor *v)
+{
+    return container_of(v, QmpInputVisitor, visitor);
+}
+
+static const 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;
+    }
+
+    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) {
+        return qlist_entry_obj(qiv->stack[qiv->nb_stack - 1].entry);
+    }
+
+    return qobj;
+}
+
+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->nb_stack++;
+
+    if (qiv->nb_stack >= QIV_STACK_SIZE) {
+        error_set(errp, QERR_BUFFER_OVERRUN);
+        return;
+    }
+}
+
+static void qmp_input_pop(QmpInputVisitor *qiv, Error **errp)
+{
+    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, const char *name, size_t size, Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    const QObject *qobj = qmp_input_get_object(qiv, name);
+
+    if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict");
+        return;
+    }
+
+    qmp_input_push(qiv, qobj, errp);
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (obj) {
+        *obj = qemu_mallocz(size);
+    }
+}
+
+static void qmp_input_end_struct(Visitor *v, Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+
+    qmp_input_pop(qiv, 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);
+
+    if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "list");
+        return;
+    }
+
+    qmp_input_push(qiv, qobj, errp);
+}
+
+static GenericList *qmp_input_next_list(Visitor *v, GenericList **list, Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+    GenericList *entry;
+    StackObject *so = &qiv->stack[qiv->nb_stack - 1];
+
+    if (so->entry == NULL) {
+        return NULL;
+    }
+
+    entry = qemu_mallocz(sizeof(*entry));
+    if (*list) {
+        so->entry = qlist_next(so->entry);
+        if (so->entry == NULL) {
+            qemu_free(entry);
+            return NULL;
+        }
+        (*list)->next = entry;
+    }
+    *list = entry;
+
+
+    return entry;
+}
+
+static void qmp_input_end_list(Visitor *v, Error **errp)
+{
+    QmpInputVisitor *qiv = to_qiv(v);
+
+    qmp_input_pop(qiv, errp);
+}
+
+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);
+
+    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer");
+        return;
+    }
+
+    *obj = qint_get_int(qobject_to_qint(qobj));
+}
+
+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);
+
+    if (!qobj || qobject_type(qobj) != QTYPE_QBOOL) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean");
+        return;
+    }
+
+    *obj = qbool_get_int(qobject_to_qbool(qobj));
+}
+
+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);
+
+    if (!qobj || qobject_type(qobj) != QTYPE_QSTRING) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "string");
+        return;
+    }
+
+    *obj = qemu_strdup(qstring_get_str(qobject_to_qstring(qobj)));
+}
+
+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);
+
+    if (!qobj || qobject_type(qobj) != QTYPE_QFLOAT) {
+        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "double");
+        return;
+    }
+
+    *obj = qfloat_get_double(qobject_to_qfloat(qobj));
+}
+
+static void qmp_input_type_enum(Visitor *v, int *obj, const char *kind, const char *name, Error **errp)
+{
+    int64_t value;
+    qmp_input_type_int(v, &value, name, errp);
+    *obj = value;
+}
+
+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);
+
+    if (!qobj) {
+        *present = false;
+        return;
+    }
+
+    *present = true;
+}
+
+static void qmp_input_end_optional(Visitor *v, Error **errp)
+{
+}
+
+Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
+{
+    return &v->visitor;
+}
+
+void qmp_input_visitor_cleanup(QmpInputVisitor *v)
+{
+    qemu_free(v);
+}
+
+QmpInputVisitor *qmp_input_visitor_new(const QObject *obj)
+{
+    QmpInputVisitor *v;
+
+    v = qemu_mallocz(sizeof(*v));
+
+    v->visitor.start_struct = qmp_input_start_struct;
+    v->visitor.end_struct = qmp_input_end_struct;
+    v->visitor.start_list = qmp_input_start_list;
+    v->visitor.next_list = qmp_input_next_list;
+    v->visitor.end_list = qmp_input_end_list;
+    v->visitor.type_enum = qmp_input_type_enum;
+    v->visitor.type_int = qmp_input_type_int;
+    v->visitor.type_bool = qmp_input_type_bool;
+    v->visitor.type_str = qmp_input_type_str;
+    v->visitor.type_number = qmp_input_type_number;
+    v->visitor.start_optional = qmp_input_start_optional;
+    v->visitor.end_optional = qmp_input_end_optional;
+
+    v->obj = obj;
+
+    return v;
+}
diff --git a/qapi/qmp-input-visitor.h b/qapi/qmp-input-visitor.h
new file mode 100644
index 0000000..0f77916
--- /dev/null
+++ b/qapi/qmp-input-visitor.h
@@ -0,0 +1,27 @@ 
+/*
+ * Input Visitor
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QMP_INPUT_VISITOR_H
+#define QMP_INPUT_VISITOR_H
+
+#include "qapi-visit-core.h"
+#include "qobject.h"
+
+typedef struct QmpInputVisitor QmpInputVisitor;
+
+QmpInputVisitor *qmp_input_visitor_new(const QObject *obj);
+void qmp_input_visitor_cleanup(QmpInputVisitor *v);
+
+Visitor *qmp_input_get_visitor(QmpInputVisitor *v);
+
+#endif
diff --git a/qerror.h b/qerror.h
index 16c830d..9a9fa5b 100644
--- a/qerror.h
+++ b/qerror.h
@@ -124,6 +124,9 @@  QError *qobject_to_qerror(const QObject *obj);
 #define QERR_JSON_PARSE_ERROR \
     "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
 
+#define QERR_BUFFER_OVERRUN \
+    "{ 'class': 'BufferOverrun', 'data': {} }"
+
 #define QERR_KVM_MISSING_CAP \
     "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"