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 this? >> >> 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. > > 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 is > usually of no concern when it's used). >
>> 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 sure >> I want to go there, though. > > 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 was >> 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? > > 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: > > foo((Parent *)child, blah) > foo(ChildBase(child), blah) > > 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. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature