Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
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 16/22] golang/xenlight: implement keyed union C to Go marshaling
Date: Wed, 4 Dec 2019 18:40:39 +0000
Message-ID: <cabd32fd-7c1e-ad7a-c4ce-1ae716894746@citrix.com> (raw)
In-Reply-To: <938dbf7c3a083ec050c16729805f4ce5f3f2891f.1573840474.git.rosbrookn@ainfosec.com>

On 11/15/19 7:44 PM, Nick Rosbrook wrote:
> From: Nick Rosbrook <rosbrookn@ainfosec.com>
> 
> Switch over union key to determine how to populate 'union' in Go struct.
> 
> Since the unions of C types cannot be directly accessed, add C structs in
> cgo preamble to assist in marshaling keyed unions. This allows the C
> type defined in the preamble to be populated first, and then accessed
> directly to populate the Go struct.

Blech. :-(

> +def xenlight_golang_union_fields_from_C(ty = None):
> +    s = ''
> +
> +    for f in ty.fields:
> +        gotypename = xenlight_golang_fmt_name(f.type.typename)
> +        ctypename  = f.type.typename
> +        gofname    = xenlight_golang_fmt_name(f.name)
> +        cfname     = f.name
> +
> +        is_castable = (f.type.json_parse_type == 'JSON_INTEGER' or
> +                       isinstance(f.type, idl.Enumeration) or
> +                       gotypename in go_builtin_types)
> +
> +        if not is_castable:
> +            s += 'if err := x.{}.fromC(&tmp.{});'.format(gofname,cfname)
> +            s += 'err != nil {\n return err \n}\n'
> +
> +        # We just did an unsafe.Pointer cast from []byte to the 'union' type
> +        # struct, so we need to make sure that any string fields are actually
> +        # converted properly.
> +        elif gotypename == 'string':
> +            s += 'x.{} = C.GoString(tmp.{})\n'.format(gofname,cfname)
> +
> +        else:
> +            s += 'x.{} = {}(tmp.{})\n'.format(gofname,gotypename,cfname)

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?


> +typedef struct libxl_channelinfo_connection_union_pty {
> +	char * path;
> +} libxl_channelinfo_connection_union_pty;

It would be nice if there were some way we could verify that the
structures generated here matched the C unions generated.  It would
stink pretty badly if they drifted and nobody noticed until we started
getting weird errors.

We don't have to solve it now, but let's put it on the to-do list and
have a think about it.

Overall looks about as good as it can -- thanks again for working this out.

 -George


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

  reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-15 19:44 [Xen-devel] [PATCH v2 00/22] generated Go libxl bindings using IDL Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 01/22] golang/xenlight: generate enum types from IDL Nick Rosbrook
2019-12-03 18:11   ` George Dunlap
2019-12-04 15:58   ` George Dunlap
2019-12-05 15:02   ` George Dunlap
2019-12-05 17:00     ` Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 02/22] golang/xenlight: define Defbool builtin type Nick Rosbrook
2019-12-04 15:50   ` George Dunlap
2019-12-05 15:23     ` Nick Rosbrook
2019-12-05 15:27       ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 03/22] golang/xenlight: define Devid type as int Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 04/22] golang/xenlight: define KeyValueList as empty struct Nick Rosbrook
2019-12-04 16:08   ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 05/22] golang/xenlight: re-name Bitmap marshaling functions Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 06/22] golang/xenlight: define StringList builtin type Nick Rosbrook
2019-12-04 16:15   ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 07/22] golang/xenlight: define Mac " Nick Rosbrook
2019-12-04 16:18   ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 08/22] golang/xenlight: define MsVmGenid " Nick Rosbrook
2019-12-04 17:00   ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 09/22] golang/xenlight: define EvLink builtin as empty struct Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 10/22] golang/xenlight: define CpuidPolicyList builtin type Nick Rosbrook
2019-12-04 16:48   ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 11/22] golang/xenlight: re-factor Uuid type implementation Nick Rosbrook
2019-12-04 17:02   ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 12/22] golang/xenlight: re-factor Hwcap " Nick Rosbrook
2019-12-04 17:07   ` George Dunlap
2019-12-05 15:35     ` Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 13/22] golang/xenlight: generate structs from the IDL Nick Rosbrook
2019-12-04 17:25   ` George Dunlap
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 14/22] golang/xenlight: remove no-longer used type MemKB Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 15/22] golang/xenlight: begin C to Go type marshaling Nick Rosbrook
2019-12-04 18:07   ` George Dunlap
2019-12-05 16:38     ` Nick Rosbrook
2019-12-05 18:00       ` George Dunlap
2019-12-05 18:32         ` Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 16/22] golang/xenlight: implement keyed union C to Go marshaling Nick Rosbrook
2019-12-04 18:40   ` George Dunlap [this message]
2019-12-05 12:22     ` George Dunlap
2019-12-05 16:53       ` Nick Rosbrook
2019-12-05 17:33         ` George Dunlap
2019-12-05 18:39           ` Nick Rosbrook
2019-12-06 10:46             ` George Dunlap
2019-12-06 15:39               ` Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 17/22] golang/xenlight: implement array " Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 18/22] golang/xenlight: begin Go to C type marshaling Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 19/22] golang/xenlight: implement keyed union Go to C marshaling Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 20/22] golang/xenlight: implement array " Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 21/22] golang/xenlight: revise use of Context type Nick Rosbrook
2019-11-15 19:44 ` [Xen-devel] [PATCH v2 22/22] golang/xenlight: add error return type to Context.Cpupoolinfo Nick Rosbrook

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cabd32fd-7c1e-ad7a-c4ce-1ae716894746@citrix.com \
    --to=george.dunlap@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=kerriganb@ainfosec.com \
    --cc=rosbrookn@ainfosec.com \
    --cc=rosbrookn@gmail.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git