On 10/23/2015 09:30 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> 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 > > Who's the client? Suggest to simply drop "if the client gets it wrong". > >> 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. > > Well, if you torture the preprocessor hard enough, it surrenders and > lets you do type checking. Torture isn't pretty, though. See for > instance > > http://git.ozlabs.org/?p=ccan;a=blob;f=ccan/check_type/check_type.h;h=77501a95597c3e73396c270d54a8ed53a9defbc4;hb=HEAD
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 = '' >> + if struct: >> + member = '->base' >> + return mcgen(''' >> + >> +static inline %(base)s *qapi_%(c_name)s_base(const %(c_name)s *obj) >> +{ >> + return (%(base)s *)obj%(member)s; > >> +} >> +''', >> + c_name=c_name(name), base=base.c_name(), member=member) > > The ugliness doesn't bother me much, because it'll go away. But perhaps > we should simply limit gen_upcast() to unions now, and extend it to > structs when we unbox their base. > 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_ext, >> + add_addr_info(qapi_SpiceServerInfo_base(server), >> + (struct sockaddr *)&info->laddr_ext, >> info->llen_ext); >> } else { >> error_report("spice: %s, extended address is expected", > > 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_abort); >> + 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); > > Not a single cast to union base? Not that I could find. So I'll have to create one in the testsuite. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature