From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44042) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpjCy-0003nt-7d for qemu-devel@nongnu.org; Fri, 23 Oct 2015 16:45:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpjCu-0000Rp-Vz for qemu-devel@nongnu.org; Fri, 23 Oct 2015 16:45:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34773) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpjCu-0000Ri-Nt for qemu-devel@nongnu.org; Fri, 23 Oct 2015 16:45:00 -0400 References: <1445576998-2921-1-git-send-email-eblake@redhat.com> <1445576998-2921-10-git-send-email-eblake@redhat.com> <87lhatbo1o.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <562A9C46.8010909@redhat.com> Date: Fri, 23 Oct 2015 14:44:54 -0600 MIME-Version: 1.0 In-Reply-To: <87lhatbo1o.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="noQPULeGiEg6xMfOdhx3fKVAMpvJiB1v5" Subject: Re: [Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi base classes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Michael Roth , qemu-devel@nongnu.org, Gerd Hoffmann This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --noQPULeGiEg6xMfOdhx3fKVAMpvJiB1v5 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/23/2015 09:30 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> A previous patch (commit 1e6c1616) made it possible to >> directly cast from a qapi type to its base type. A future >> patch will do likewise for structs. However, it requires >> the client code to use a C cast, which turns off compiler >> type-safety checks if the client gets it wrong. So this >=20 > Who's the client? Suggest to simply drop "if the client gets it wrong"= =2E >=20 >> patch adds inline type-safe wrappers named qapi_FOO_base() >> for any type FOO that has a base, which can be used to >> upcast a qapi type to its base, then uses the new generated >> functions in places where we were already casting. >> >> Some of the ugliness of this patch will disappear once >> structs are laid out in the same manner as unions; mark >> it with TODO for now. >> >> Note that C makes const-correct upcasts annoying because >> it lacks overloads; these functions cast away const so that >> they can accept user pointers whether const or not, and the >> result in turn can be assigned to normal or const pointers. >> Alternatively, this could have been done with macros, but >> those tend to not have quite as much type safety. >=20 > Well, if you torture the preprocessor hard enough, it surrenders and > lets you do type checking. Torture isn't pretty, though. See for > instance >=20 > http://git.ozlabs.org/?p=3Dccan;a=3Dblob;f=3Dccan/check_type/check_type= =2Eh;h=3D77501a95597c3e73396c270d54a8ed53a9defbc4;hb=3DHEAD Oddly enough, our container_of() macro looks verrrrry similar to the comments in that sample code (that is, we DO use the preprocessor for a type check). :) >> +++ b/scripts/qapi-types.py >> @@ -97,6 +97,23 @@ struct %(c_name)s { >> return ret >> >> >> +def gen_upcast(name, base, struct): >> + # C makes const-correctness ugly. We have to cast away const to = let >> + # this function work for both const and non-const obj. >> + # TODO Ugly difference between struct and flat union bases >> + member =3D '' >> + if struct: >> + member =3D '->base' >> + return mcgen(''' >> + >> +static inline %(base)s *qapi_%(c_name)s_base(const %(c_name)s *obj) >> +{ >> + return (%(base)s *)obj%(member)s; >=20 >> +} >> +''', >> + c_name=3Dc_name(name), base=3Dbase.c_name(), member=3D= member) >=20 > The ugliness doesn't bother me much, because it'll go away. But perhap= s > we should simply limit gen_upcast() to unions now, and extend it to > structs when we unbox their base. >=20 Okay, I could do that. Then I definitely need to post a v11 respin, this is starting to be too much for simple fixups to v10. >> - add_addr_info(server->base, (struct sockaddr *)&info->laddr_e= xt, >> + add_addr_info(qapi_SpiceServerInfo_base(server), >> + (struct sockaddr *)&info->laddr_ext, >> info->llen_ext); >> } else { >> error_report("spice: %s, extended address is expected", >=20 > This change will start to make sense when we unbox base. Right now, it= > doesn't :) Indeed, if I don't port gen_upcast() to types until patch 10/25, then this hunk also has to move to patch 10. That means no clients of the upcast macros in patch 9/25, _unless_ I add testsuite coverage. Which I probably ought to do. >> switch (event) { >> case QAPI_EVENT_VNC_CONNECTED: >> - qapi_event_send_vnc_connected(si, vs->info->base, &error_abor= t); >> + qapi_event_send_vnc_connected(si, qapi_VncClientInfo_base(vs-= >info), >> + &error_abort); >> break; >> case QAPI_EVENT_VNC_INITIALIZED: >> qapi_event_send_vnc_initialized(si, vs->info, &error_abort); >=20 > Not a single cast to union base? Not that I could find. So I'll have to create one in the testsuite. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --noQPULeGiEg6xMfOdhx3fKVAMpvJiB1v5 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/ iQEcBAEBCAAGBQJWKpxGAAoJEKeha0olJ0NqKBYIAIoiYzwXueyJKpiJ7CW0WZPE 2HJoCZfJcVJnZwp/bCw3TH6SA1zjmXFQJyrvhWzNr2vXhflf3zvfvH3Yvns1hfjx 5yj9a1GeacX3yhMGOfFusvko18EDBQlYg2mVOo/JehsQH/FnTnPY0fDZojbLTssh JqPdE2weoGSsTro3NhFrnJ34H2RttI4LHtq4BeGOr1cVDhrMKPzSGJR90ZhEjjbH ZqrzN0oBlovdsCj3BL8uK4qoxiTMxs5bXAQibnoWRNgzgkPhSNAwQAbfa31aJP6g 0fYzDBXhRrwdmPtiYSDYYjpaAMDRYf8I9R9PaOEFchSxdwriELNyRjNTWyqDb8w= =UGym -----END PGP SIGNATURE----- --noQPULeGiEg6xMfOdhx3fKVAMpvJiB1v5--