From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:35405)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1buaMH-00042T-1c
	for qemu-devel@nongnu.org; Thu, 13 Oct 2016 03:23:20 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1buaMB-0000ks-Gm
	for qemu-devel@nongnu.org; Thu, 13 Oct 2016 03:23:12 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1475246744-29302-1-git-send-email-berrange@redhat.com>
	<1475246744-29302-9-git-send-email-berrange@redhat.com>
	<87d1j6yopf.fsf@dusky.pond.sub.org>
Date: Thu, 13 Oct 2016 09:23:03 +0200
In-Reply-To: <87d1j6yopf.fsf@dusky.pond.sub.org> (Markus Armbruster's message
	of "Wed, 12 Oct 2016 10:08:12 +0200")
Message-ID: <877f9cg1bc.fsf@dusky.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v14 08/21] qapi: allow QObjectInputVisitor
 to be created with QemuOpts
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Daniel P. Berrange" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Andreas =?utf-8?Q?F=C3=A4rber?= <afaerber@suse.de>

Markus Armbruster <armbru@redhat.com> writes:

> "Daniel P. Berrange" <berrange@redhat.com> 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 <berrange@redhat.com>
[...]
>> 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;
>> +}
[...]