Re: [Xen-devel] [PATCH v2 15/22] golang/xenlight: begin C to Go type marshaling

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 12/5/19 4:38 PM, Nick Rosbrook wrote:
>> You should probably say here explicitly what kinds of elements you're
>> supporting and not supporting in this patch; specifically:
>>
>> - You're converting built-ins (or is this any struct-like type?)
> 
> Any struct-like type, since the fromC functions are all defined in
> this patch (excluding array fields and keyed unions as you said
> below).
> 
>> - You handle nested anonymous structs
>> - But you're not handling keyed unions or arrays (anything else)?
> 
> I think this covers it, thanks.
> 
>>> +func (x *VncInfo) fromC(xc *C.libxl_vnc_info) error {
>>> +     var defboolEnable Defbool
>>> +     if err := defboolEnable.fromC(&xc.enable); err != nil {
>>> +             return err
>>> +     }
>>
>> Is there a reason in these cases that we don't simply call .fromC on the
>> elemet itself?
> 
> This ensures that when we call fromC, we have an initialized variable.
> This might be overkill here, as this would matter more if we had
> structs with a nested struct pointer. E.g., [1] will panic since only
> the outer struct is initialized.

So first of all, I noticed that the marshalling code for Union structs
does what I suggest. :-)

I can see how such a construct would be needed when there was a pointer
type.  But if there was a pointer type, you'd have to special-case
things anyway.  Take the following code that has a non-pointer element:

        var defboolEnable Defbool // The same type as x.Enable
        if err := defboolEnable.fromC(&xc.enable); err != nil {
                return err
        }
        x.Enable = defboolEnable

Now suppose you had a pointer element instead; what would it look like?

        var defboolEnable Defbool // NOT the same type as x.Enable!
        if err := defboolEnable.fromC(&xc.enable); err != nil {
                return err
        }
        x.Enable = &defboolEnable

The generation code would have to replace the type of "Enable", which is
a pointer, with the actual instance of the thing; and then assign the
reference, rather than the value.  So if we run across something like
that, we can sort that out when we come to it.

I'm not super-strong on this, so I don't want to bike-shed.  But I think
the way you generate the marshalling code for the union structs is better:

        if err := x.Pae.fromC(&tmp.pae); err != nil {
                return err
        }

Thanks,
 -George

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