From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38454) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aygRH-00008Z-8O for qemu-devel@nongnu.org; Fri, 06 May 2016 10:09:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aygR5-0008Ru-9i for qemu-devel@nongnu.org; Fri, 06 May 2016 10:09:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35146) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aygR5-0008Lv-22 for qemu-devel@nongnu.org; Fri, 06 May 2016 10:08:55 -0400 References: <1461903820-3092-1-git-send-email-eblake@redhat.com> <1461903820-3092-8-git-send-email-eblake@redhat.com> <87lh3som6o.fsf@dusky.pond.sub.org> <57276E26.5020006@redhat.com> <87r3dja6ue.fsf@dusky.pond.sub.org> <87wpn9esj5.fsf@dusky.pond.sub.org> <572C1AB8.1060705@redhat.com> <87k2j74bbc.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <572CA569.1000702@redhat.com> Date: Fri, 6 May 2016 08:08:41 -0600 MIME-Version: 1.0 In-Reply-To: <87k2j74bbc.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gE6aX8kIOR3jql5e8m7c76xeTmVAjFmWP" Subject: Re: [Qemu-devel] [PATCH v3 07/18] qapi: Add json output visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: famz@redhat.com, qemu-devel@nongnu.org, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --gE6aX8kIOR3jql5e8m7c76xeTmVAjFmWP Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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? >=20 > The two output functions are >=20 > QObject *qmp_output_get_qobject(QmpOutputVisitor *v); > char *string_output_get_string(StringOutputVisitor *v); >=20 >> 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 m= ay >> be easier to just patch the testsuite to use a new visitor in the plac= es >> where it currently does a reset. >=20 > 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 =3D NULL; >> Error *local_err =3D NULL; >> Visitor *v; >> >> v =3D 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= =2E >=20 > I like not having to deal with the QmpOutputVisitor type, but like you,= > I don't like action at a distance. >=20 >> Maybe we need to keep the FOO_new() functions returning FOO* rather >> than Visitor*, along with the FOO_get_visitor() functions, after all. >=20 > I can think of a other ways to hide the QmpOutputVisitor type, but they= > have drawbacks, too. >=20 > 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. >=20 > 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 =3D qmp_output_visitor_new(&ret); =2E.. visit_complete(v, &ret); where the completion function asserts that opaque matches the same pointer as passed in with correct type during new(). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --gE6aX8kIOR3jql5e8m7c76xeTmVAjFmWP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXLKVpAAoJEKeha0olJ0NqL/0H/Ro+l1iB6GGBRvEzAwCXKBfn L5rn7ehU40WcQoQD+pV+jo/GmHCEN2LWX5oGC4zekCbKel8xu3WZwzsFY+vF/wMJ M4kwZGX87/g8TtgXuXKuBFYCNOrvhi8LSgBgzF5aPMsJWFTL+/HTsosimOz6I4vC kfxCfOt3HzWu1v7VI902C0w2IiMRgIluWKEHhX/GySS2NSe7KBv8wCLLlCMFL31w xysHIgalEMdOPW2aLzEMuHQAYCmfgjNnn2Cg4Nm59dVViA2tfMNSZPD731zdzbDj NRQpyEOsP9wKBjW+QqArzg+lx6VF45AfgMRiZIhsgEsd2DMXP3git8ctpNDSHlY= =jrP/ -----END PGP SIGNATURE----- --gE6aX8kIOR3jql5e8m7c76xeTmVAjFmWP--