On 10/27/2015 01:46 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> A previous patch (commit 1e6c1616) made it possible to >> directly cast from a qapi flat union type to its base type. >> However, it requires the use of a C cast, which turns off >> compiler type-safety checks. Add inline type-safe wrappers >> named qapi_FOO_base() for any union type FOO that has a base, >> which can be used for a safer upcast, and enhance the >> testsuite to cover the new functionality. A future patch >> will extend the upcast support to structs. >> >> 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, the macros can be made just as type-safe, but the result is either > somewhat ugly and using gcc-isms, or very ugly and unhygienic. > > I'd write something like "type-safe macros are hairy, and not worthwhile > here."
Sure, that works for me. > >> This patch just adds upcasts. None of our code needed to >> downcast from a base qapi class to a child. > > Actually, none of our code needs to upcast unions, either. Only the new > tests do. Code that updasts structs exist, but it doesn't use this > patch's upcasts until later. > > Suggest to amend the first paragraph: > > A previous patch (commit 1e6c1616) made it possible to directly cast > from a qapi flat union type to its base type. However, it requires > the use of a C cast, which turns off compiler type-safety checks. > Fortunately, no such casts exist just, yet. s/ just,/, just/ > > Regardless, add inline type-safe wrappers named qapi_FOO_base() for > any union type FOO that has a base, which can be used for a safer > upcast, and enhance the testsuite to cover the new functionality. > > A future patch will extend the upcast support to structs, where such > casts do exist already. > Maybe s/casts/conversions/ - because as of this patch, it is still a conversion via foo->base rather than (Base *)foo (it's the next patch that gets rid of base, and therefore needs either the cast or the wrapper). > > Patch looks good. I can touch up the commit message in my tree. Sure, your proposed wording + touchups is fine. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
