From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49599) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zp0ky-0000RW-Ht for qemu-devel@nongnu.org; Wed, 21 Oct 2015 17:17:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zp0kw-0001Jb-S3 for qemu-devel@nongnu.org; Wed, 21 Oct 2015 17:17:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51957) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zp0kw-0001JU-LJ for qemu-devel@nongnu.org; Wed, 21 Oct 2015 17:17:10 -0400 References: <1444968943-11254-1-git-send-email-eblake@redhat.com> <1444968943-11254-6-git-send-email-eblake@redhat.com> <8737x591xz.fsf@blackfin.pond.sub.org> <56266704.2060403@redhat.com> <87pp08ny5g.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <562800CB.10108@redhat.com> Date: Wed, 21 Oct 2015 15:16:59 -0600 MIME-Version: 1.0 In-Reply-To: <87pp08ny5g.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bmV9v6bQBwWAh1V30f7wNQrEGPMfdU2o9" Subject: Re: [Qemu-devel] [PATCH v9 05/17] qapi: Unbox base members List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Michael Roth , Gerd Hoffmann , Luiz Capitulino This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --bmV9v6bQBwWAh1V30f7wNQrEGPMfdU2o9 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/21/2015 07:34 AM, Markus Armbruster wrote: >>>> @@ -218,9 +216,11 @@ static void channel_event(int event, >>>> SpiceChannelEventInfo *info) >>>> } >>>> >>>> if (info->flags & SPICE_CHANNEL_EVENT_FLAG_ADDR_EXT) { >>>> - add_addr_info(client->base, (struct sockaddr *)&info->paddr= _ext, >>>> + add_addr_info((SpiceBasicInfo *)client, >>> >>> Ah, you're already exploiting the ability to cast to the base type! >> >> Absolutely :) >> >>> >>> Idea: should we generate a type-safe macro or inline function for thi= s? >> >> Hmm. DO_UPCAST() (and its more powerful underlying container_of()) >> doesn't fit here, because we inlined the fields rather than directly >> including the base. >=20 > Yes, because it results in slightly more readable code: always simply > p->m instead of things like p->base.base.m when m is inherited (which i= s > usually of no concern when it's used). >=20 >> There's also the ugly approach of exposing things in a dual naming >> system via an anonymous union and struct: >> >> struct Child { >> union { >> struct { >> int i; >> }; >> Parent base; >> }; >> bool b; >> }; >> >> which would allow 'child->i' to be the same storage as 'child->base.i'= , >> so that clients can use the short spelling when accessing fields but >> also have handy access to the base member for DO_UPCAST(). I'm not su= re >> I want to go there, though. >=20 > Seems to clever for its own sake :) I agree (and even though I'm using a similar hack in 7/17 for the same purpose, I get rid of it as soon as possible in 16/17). >> But while such a representation would add compiler type-safety (hiding= >> the cast in generated code, where we can prove the generator knew it w= as >> safe, and so that clients don't have to cast), it also adds verbosity.= >> I can't think of any representation that would be shorter than making >> the clients do the cast, short of using a container rather than inline= >> approach. Even foo(qapi_baseof_Child(child), blah) is longer than >> foo((Parent *)child, blah). >> >> Preferences? >=20 > The least verbose naming convention for a conversion function I can > think of right now is TBase(), where T is the name of a type with a > base. Compare: >=20 > foo((Parent *)child, blah) > foo(ChildBase(child), blah) >=20 > Tolerable? Worthwhile? The verbosity is then determined by how long the child name is (where the cast depended on the parent name) What happens with multiple inheritance? If we have Grandparent -> Parent -> Child, then it should be possible to cast to both bases: (Grandparent *)child (Parent *)child with your scheme, getting a child to grandparent would have to look like either of: ParentBase(ChildBase(child)) ChildBaseBase(child) Or, think what happens if we originally have Grandparent -> Child in one version of qemu, then inject Parent in the middle in another - the QMP can still be back-compat, and the direct casts and member variable references in C still work, but any code using ChildBase() no longer works (returning Parent* instead of Grandparent*). So the only thing I can think of is to output some name that includes both the child type and parent type name (to make it obvious which conversion is being done): qapi_Child_Grandparent(child) qapi_Child_Parent(child) At this point, I'm leaning towards client casts, just because of the verbosity, but I'll at least try the generated type-safe functions in v10 to see how bad it really is. A patch to discuss will make it easier to decide whether to paint this bikeshed. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --bmV9v6bQBwWAh1V30f7wNQrEGPMfdU2o9 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/ iQEcBAEBCAAGBQJWKADLAAoJEKeha0olJ0Nqa0sH+wV1WEOlqEzREdS4FBfQD/vg L1hbaRSZnxpb4D7n6FY9h2g2LTKkb+UhYrEvfHyfQWAcTB/YQp4aUXl1CTQdOl3N LPwsZEHeCr4P5E5YbUGthK4GsRKAD1m3wXZsR7oqSVcKnIm8CpTIp95rbYkl/VFY Ik1ewBdLzVlQMMF7/t2aX9bkf+50dSQIvEeM5GIk7RDRiQHfdhIqnK8x52qEu864 M/pYt87/NueXFOERU3pHrKX/WvPmt/Y3p5cc4lRNbzMbvak26GdX35jcVbCskwOn 3cvdXz2l9Ww8tQnNKLLSNOy3ksEBs/nEr8o1HFIvTq9DE3xLfJiUrq3Hwtj7uAI= =SuOx -----END PGP SIGNATURE----- --bmV9v6bQBwWAh1V30f7wNQrEGPMfdU2o9--