Re: [Xen-devel] [PATCH v2] golang/xenlight: implement constructor generation

[Nick Rosbrook <rosbrookn@xxxxxxxxx>]

> NewDomainConfig() as of this patch can never return success, because
> DomainConfig.fromC() will call DomainBuildInfo.fromC(), which will choke
> on b_info.type = LIBXL_DOMAIN_TYPE_INVALID.
>
> This is actually a bug in to/fromC.  Consider libxl_channelinfo.
>
> The idl says:
>
> libxl_channelinfo = Struct("channelinfo", [
>     ("backend", string),
>     ("backend_id", uint32),
>     ("frontend", string),
>     ("frontend_id", uint32),
>     ("devid", libxl_devid),
>     ("state", integer),
>     ("evtch", integer),
>     ("rref", integer),
>     ("u", KeyedUnion(None, libxl_channel_connection, "connection",
>            [("unknown", None),
>             ("pty", Struct(None, [("path", string),])),
>             ("socket", None),
>            ])),
>     ], dir=DIR_OUT)
>
> But the generated code currently only generates:
>
> type Channelinfo struct {
>         Backend         string
>         BackendId       uint32
>         Frontend        string
>         FrontendId      uint32
>         Devid           Devid
>         State           intWhich means if libxl passes back
>         Evtch           int
>         Rref            int
>         Connection      ChannelConnection
>         ConnectionUnion channelinfoConnectionUnion
> }
>
> type channelinfoConnectionUnion interface {
>         ischannelinfoConnectionUnion()
> }
>
> type ChannelinfoConnectionUnionPty struct {
>         Path string
> }
>
> func (x ChannelinfoConnectionUnionPty) ischannelinfoConnectionUnion() {}
>
> I think this makes sense -- there's no need to have types for 'unknown'
> and 'socket' just to hold nothing.  But then the marshaling code looks
> like this:
>
>         switch x.Connection {
>         case ChannelConnectionPty:
>                 tmp, ok := x.ConnectionUnion.(ChannelinfoConnectionUnionPty)
>                 if !ok {
>                         return errors.New("wrong type for union key 
> connection")
>                 }
>                 var pty C.libxl_channelinfo_connection_union_pty
>                 if tmp.Path != "" {
>                         pty.path = C.CString(tmp.Path)
>                 }
>                 ptyBytes := C.GoBytes(unsafe.Pointer(&pty),
> C.sizeof_libxl_channelinfo_connection_union_pty)
>                 copy(xc.u[:], ptyBytes)
>         default:
>                 return fmt.Errorf("invalid union key '%v'", x.Connection)
>         }
>
> So this will incorrectly fail for for either 'unknown' or 'socket'.
> What we need to have is for toC to ignore enumerated values that have
> empty types, and fromC to set the union to `nil` in these cases.
>
> I've got a patch -- I'll send it out.

Oh, yeah I see the problem in xenlight_golang_union_from_C -- it just
continues in the loop if f.type is None to avoid defining another
fromC, but should still should add a case in the switch statement.

Good catch, thanks.

-NR

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