Re: [RESEND PATCH 04/12] golang/xenlight: export keyed union interface types

From: George Dunlap <George.Dunlap@citrix.com>
To: Nick Rosbrook <rosbrookn@gmail.com>
Cc: "xen-devel@lists.xenproject.prg" <xen-devel@lists.xenproject.prg>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Nick Rosbrook" <rosbrookn@ainfosec.com>,
	Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>
Subject: Re: [RESEND PATCH 04/12] golang/xenlight: export keyed union interface types
Date: Fri, 18 Jun 2021 11:19:55 +0000	[thread overview]
Message-ID: <A972F792-5327-438B-BA92-DB5A334AAA3E@citrix.com> (raw)
In-Reply-To: <29a3fbc93262cb9b31f02d6c94c018b200dfa43e.1621887506.git.rosbrookn@ainfosec.com>

> On May 24, 2021, at 9:36 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> 
> For structs that have a keyed union, e.g. DomainBuildInfo, the TypeUnion
> field must be exported so that package users can get/set the fields
> within. This means that users are aware of the existence of the
> interface type used in those fields (see [1]), so it is awkward that the
> interface itself is not exported. However, the single method within the
> interface must remain unexported so that users cannot mistakenly "implement"
> those interfaces.
> 
> Since there seems to be no reason to do otherwise, export the keyed
> union interface types.
> 
> [1] https://pkg.go.dev/xenbits.xenproject.org/git-http/xen.git/tools/golang/xenlight?tab=doc#DeviceUsbdev
> 
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

I wonder if at some point we should add documentation to the definitions of specific union types, saying explicitly what union type it implements, and maybe what the Type field should be set to.  e.g.:

DeviceUsbdevTypeUnionHostdev implements the DeviceUsbdevTypeUnion interface.  If DeviceUsbdev.TypeUnion is set to this type, DeviceUsbdev.Type should be set to UsbdevTypeHostdev.

Might be overkill tho.