Re: [Qemu-devel] [PATCH v3 05/21] qapi: add QMP input visitor

[Michael Roth]

On 06/15/2011 11:11 AM, Luiz Capitulino wrote:
> On Mon, 13 Jun 2011 21:31:10 -0500
> Michael Roth<address@hidden>  wrote:
<snip>
> > +Visitor *qmp_input_get_visitor(QmpInputVisitor *v)
> > +{
> > +    return&v->visitor;
> > +}
> > +
> > +void qmp_input_visitor_cleanup(QmpInputVisitor *v)
> > +{
>
> You're not decrementing the reference to v->obj. Actually, we should also
> increment the reference to it before storing or document that the reference
> ownership is transferred to this function.

Ownership never gets transferred to the visitor in the dispatch path; 
the caller is supposed to free it. I thought there was some manipulation 
elsewhere that prevented us from using a const argument to 
qmp_input_visitor_new() instead, but it looks like we can. Otherwise, 
I'd prefer to do an inc+dec to make this more explicit.

> > +    qemu_free(v);
> > +}
> > +
> > +QmpInputVisitor *qmp_input_visitor_new(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..3f798f0
> > --- /dev/null
> > +++ b/qapi/qmp-input-visitor.h
> > @@ -0,0 +1,27 @@
> > +/*
> > + * Input Visitor
> > + *
> > + * Copyright IBM, Corp. 2011
> > + *
> > + * Authors:
> > + *  Anthony Liguori<address@hidden>
> > + *
> > + * 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(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..7a89a50 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_QAPI_VISITOR_STACK_OVERRUN \
> > +    "{ 'class': 'QAPIVisitorStackOverrun', 'data': {} }"
> > +
> >   #define QERR_KVM_MISSING_CAP \
> >       "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } 
> > }"