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

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On 06/15/2011 11:11 AM, Luiz Capitulino wrote:
> On Mon, 13 Jun 2011 21:31:10 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  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<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(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 } }"
>>
>