Re: [Qemu-devel] [PATCH v14 08/21] qapi: allow QObjectInputVisitor to be created with QemuOpts

[Markus Armbruster]

Markus Armbruster <address@hidden> writes:

> "Daniel P. Berrange" <address@hidden> writes:
>
>> Instead of requiring all callers to go through the mutli-step
>
> multi-step
>
>> process of turning QemuOpts into a suitable QObject for visiting,
>> add a new constructor that encapsulates this logic. This will
>> allow QObjectInputVisitor to be a drop-in replacement for the
>> existing OptsVisitor with minimal code changes for callers.
>>
>> NB, at this point it is only supporting opts syntax which
>> explicitly matches the QAPI schema structure, so is not yet
>> a true drop-in replacement for OptsVisitor. The patches that
>> follow will add the special cases requird for full backwards
>> compatibility with OptsVisitor.
>>
>> Signed-off-by: Daniel P. Berrange <address@hidden>
[...]
>> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
>> index cf41df6..d9269c9 100644
>> --- a/qapi/qobject-input-visitor.c
>> +++ b/qapi/qobject-input-visitor.c
>> @@ -545,3 +545,28 @@ Visitor *qobject_input_visitor_new_autocast(QObject 
>> *obj)
>>  
>>      return &v->visitor;
>>  }
>> +
>> +
>> +Visitor *qobject_input_visitor_new_opts(const QemuOpts *opts,
>> +                                        Error **errp)
>> +{
>> +    QDict *pdict;
>> +    QObject *pobj = NULL;
>
> @pdict and @pobj are unusual names.  Let's stick to the more common
> @dict and @obj.
>
>> +    Visitor *v = NULL;
>> +
>> +    pdict = qemu_opts_to_qdict(opts, NULL);
>> +    if (!pdict) {
>> +        goto cleanup;

Returns null without setting an error, which is wrong.  Fortunately,
qemu_opts_to_qdict() cannot fail.  Please drop the broken error
handling.

>> +    }
>> +
>> +    pobj = qdict_crumple(pdict, true, errp);
>> +    if (!pobj) {
>> +        goto cleanup;
>> +    }
>> +
>> +    v = qobject_input_visitor_new_autocast(pobj);
>> + cleanup:
>> +    qobject_decref(pobj);
>> +    QDECREF(pdict);
>> +    return v;
>> +}
[...]