Re: [Xen-devel] [PATCH v2 16/22] golang/xenlight: implement keyed union C to Go marshaling

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 12/5/19 4:53 PM, Nick Rosbrook wrote:
>>> It looks like this is duplicating (differently!) the field-copying code
>>> from golang_define_from_C.  Is there any reason you couldn't have a
>>> single function, `xenlight_golang_fields_from_C`, which would be used
>>> for both?
> 
> No, I should be able to re-factor that. Thanks.
> 
>> Actually, it turns out we don't strictly need to duplicate this at all,
>> if we use the `typeof` operator, like this:
>>
>> ---
>> typedef typeof(((struct libxl_channelinfo *)NULL)->u.pty)
>> libxl_channelinfo_connection_union_pty;
>>
>> typedef typeof(((struct libxl_domain_build_info *)NULL)->u.hvm)
>> libxl_domain_build_info_type_union_hvm;
>>
>> typedef typeof(((struct libxl_domain_build_info *)NULL)->u.pv)
>> libxl_domain_build_info_type_union_pv;
>>
>> typedef typeof(((struct libxl_domain_build_info *)NULL)->u.pvh)
>> libxl_domain_build_info_type_union_pvh;
>>
>> typedef typeof(((struct libxl_device_usbdev *)NULL)->u.hostdev)
>> libxl_device_usbdev_type_union_hostdev;
>>
>> typedef typeof(((struct libxl_device_channel *)NULL)->u.socket)
>> libxl_device_channel_connection_union_socket;
>> ---
>>
>> This guarantees we'll have the correct layout for the resulting type.
> 
> Well that's pretty cool.
> 
>> I talked to Ian Jackson, and he agreed that long-term it would be good
>> for the C generator to generate named types for these union elements
>> (likke you have here).  If you felt really motivated you could do that
>> now; but I think using the `typeof` trick would be suitable to get this
>> patch in.
> 
> I'll take a look at least and see if I can get it done fairly easily.
> Otherwise, I'll use this trick.

It actually occurs to me that the "named struct elements of union" would
still technically open up a window for divergence: i.e., if somehow the
type of the named struct didn't match up with the union element.

I.e., the following *shouldn't* happen, but technically it *could*:

----
struct libxl_domain_build_info_union_hvm {
 ...
}

struct libxl_domain_build_info {
  union {
    libxl_domain_struct_build_info_hvm2 hvm;
  } u;
}
---

Using the `typeof` trick above guarantees that the types the marshaling
functions are using are identical to the types actually specified in the
union itself.  Particularly as this is just generated code nobody's
going to look at, I'm inclined to think the cost is near-zero.  Since
the benefit is non-zero, I'd be inclined to say just go with that instead.

And it's easier!

Thoughts?

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel