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

From: George Dunlap <george.dunlap@citrix.com>
To: Nick Rosbrook <rosbrookn@gmail.com>, <xen-devel@lists.xenproject.org>
Cc: Nick Rosbrook <rosbrookn@ainfosec.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	kerriganb@ainfosec.com, Wei Liu <wl@xen.org>
Subject: Re: [Xen-devel] [PATCH v2 15/22] golang/xenlight: begin C to Go type marshaling
Date: Wed, 4 Dec 2019 18:07:15 +0000	[thread overview]
Message-ID: <0ab13f6e-2dcf-ebbb-4d14-ee87c816701d@citrix.com> (raw)
In-Reply-To: <472334f58bc435e505ad6bc392426960630fb060.1573840474.git.rosbrookn@ainfosec.com>

On 11/15/19 7:44 PM, Nick Rosbrook wrote:
> From: Nick Rosbrook <rosbrookn@ainfosec.com>
> 
> Implement basic type conversion in fromC functions such as strings and
> integer types. Also, remove existing toGo functions from xenlight.go in
> favor of the new generated functions.

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?)
- You handle nested anonymous structs
- But you're not handling keyed unions or arrays (anything else)?

> +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?

i.e., why not just:

> +	x.Enable = defboolEnable

    x.Enable.fromC(&xc.enable)

?

Everything else looks good, thanks

 -George

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