[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 07/18] qapi: Add json output visitor

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v3 07/18] qapi: Add json output visitor
Date: Fri, 6 May 2016 08:08:41 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 05/06/2016 06:31 AM, Markus Armbruster wrote:
>>  So all that's left are the two output functions.  Can we get rid
>> of those, and make Visitor* the only public interface, rather than
>> making every caller have to do upcasts?
> The two output functions are
>     QObject *qmp_output_get_qobject(QmpOutputVisitor *v);
>     char *string_output_get_string(StringOutputVisitor *v);
>> It looks like outside of the testsuite, all uses of these visitors are
>> local to a single function; and improving the testsuite is not the end
>> of the world.  Particularly if only the testsuite is using reset, it may
>> be easier to just patch the testsuite to use a new visitor in the places
>> where it currently does a reset.
> I'm okay with replacing reset by destroy + new in the test suite.

That part's (relatively) easy, so it will be in the next spin.

>> QObject *object_property_get_qobject(Object *obj, const char *name,
>>                                      Error **errp)
>> {
>>     QObject *ret = NULL;
>>     Error *local_err = NULL;
>>     Visitor *v;
>>     v = qmp_output_visitor_new(&ret);
>>     object_property_get(obj, v, name, &local_err);
>>     if (!local_err) {
>>         visit_complete(v); /* populates ret */
>>     }
>>     error_propagate(errp, local_err);
>>     visit_free(v);
>>     return ret;
>> }
>> Slightly shorter, but populating 'ret' at a distance feels a bit weird.
> I like not having to deal with the QmpOutputVisitor type, but like you,
> I don't like action at a distance.
>>  Maybe we need to keep the FOO_new() functions returning FOO* rather
>> than Visitor*, along with the FOO_get_visitor() functions, after all.
> I can think of a other ways to hide the QmpOutputVisitor type, but they
> have drawbacks, too.
> You can let the two output functions take Visitor *.  Drawback: now the
> compiler lets you pass the wrong kind of visitor.

But at least you can assert that the right visitor was used.

> You can let visit_complete() return the output (if any) as void *.
> Drawback: now the compiler lets you misinterpret the output's type.

And you lose any chance to assert things.

Another (hybrid?) option:

void visit_complete(Visitor *v, void *opaque);
Visitor *qmp_output_visitor_new(QObject **ret);

called as:

v = qmp_output_visitor_new(&ret);
visit_complete(v, &ret);

where the completion function asserts that opaque matches the same
pointer as passed in with correct type during new().

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]