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

2019-12-16 Thread George Dunlap
On 12/10/19 3:47 PM, Nick Rosbrook wrote:
> From: Nick Rosbrook 
> 
> Begin implementation of fromC marshaling functions for generated struct
> types. This includes support for converting fields that are basic
> primitive types such as string and integer types, nested anonymous
> structs, nested libxl structs, and libxl built-in types.
> 
> This patch does not implement conversion of arrays or keyed unions.
> 
> Signed-off-by: Nick Rosbrook 
> ---
> Changes in v2:
> - Add Makefile changes for helpers.gen.go.
> - Re-generate helpers.gen.go to include libxl changes after rebase.
> Changes in v3:
> - Break out field copying/type conversion code into its own function
>   called xenlight_golang_convert_from_C to allow that code to be easily
>   re-used.
> - Use consistent style for calling fromC on struct fields that require
>   it. Namely, do not use a temporary variable - call fromC directly on
>   the struct field.

Looks good!  One minor comment...

> ---
>  tools/golang/xenlight/Makefile   |   2 +
>  tools/golang/xenlight/gengotypes.py  | 118 
>  tools/golang/xenlight/helpers.gen.go | 901 +++
>  tools/golang/xenlight/xenlight.go| 111 +---
>  4 files changed, 1032 insertions(+), 100 deletions(-)
>  create mode 100644 tools/golang/xenlight/helpers.gen.go
> 
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> index 681f32c234..07b8896e5b 100644
> --- a/tools/golang/xenlight/Makefile
> +++ b/tools/golang/xenlight/Makefile
> @@ -19,6 +19,7 @@ $(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: %.gen.go
>   $(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
>   $(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
>   $(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
> + $(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)



I'll have to edit this stanza on check-in to depend on helpers.gen.go
directly; with that:

Reviewed-by: George Dunlap 

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

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

2019-12-10 Thread Nick Rosbrook
From: Nick Rosbrook 

Begin implementation of fromC marshaling functions for generated struct
types. This includes support for converting fields that are basic
primitive types such as string and integer types, nested anonymous
structs, nested libxl structs, and libxl built-in types.

This patch does not implement conversion of arrays or keyed unions.

Signed-off-by: Nick Rosbrook 
---
Changes in v2:
- Add Makefile changes for helpers.gen.go.
- Re-generate helpers.gen.go to include libxl changes after rebase.
Changes in v3:
- Break out field copying/type conversion code into its own function
  called xenlight_golang_convert_from_C to allow that code to be easily
  re-used.
- Use consistent style for calling fromC on struct fields that require
  it. Namely, do not use a temporary variable - call fromC directly on
  the struct field.
---
 tools/golang/xenlight/Makefile   |   2 +
 tools/golang/xenlight/gengotypes.py  | 118 
 tools/golang/xenlight/helpers.gen.go | 901 +++
 tools/golang/xenlight/xenlight.go| 111 +---
 4 files changed, 1032 insertions(+), 100 deletions(-)
 create mode 100644 tools/golang/xenlight/helpers.gen.go

diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index 681f32c234..07b8896e5b 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -19,6 +19,7 @@ $(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: %.gen.go
$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
$(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
$(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+   $(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
 
 %.gen.go: gengotypes.py $(XEN_ROOT)/tools/libxl/libxl_types.idl 
$(XEN_ROOT)/tools/libxl/idl.py
XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py ../../libxl/libxl_types.idl
@@ -39,6 +40,7 @@ install: build
$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)xenlight.go 
$(DESTDIR)$(GOXL_INSTALL_DIR)
$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)types.gen.go 
$(DESTDIR)$(GOXL_INSTALL_DIR)
+   $(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)helpers.gen.go 
$(DESTDIR)$(GOXL_INSTALL_DIR)
 
 .PHONY: uninstall
rm -rf $(DESTDIR)$(GOXL_INSTALL_DIR)
diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index 8963b14eee..1fe56179e2 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -18,6 +18,12 @@ builtin_type_names = {
 idl.uint64.typename: 'uint64',
 }
 
+# Some go keywords that conflict with field names in libxl structs.
+go_keywords = ['type', 'func']
+
+go_builtin_types = ['bool', 'string', 'int', 'byte',
+'uint16', 'uint32', 'uint64']
+
 def xenlight_golang_generate_types(path = None, types = None, comment = None):
 """
 Generate a .go file (types.gen.go by default)
@@ -176,6 +182,116 @@ def xenlight_golang_define_union(ty = None, structname = 
''):
 
 return (s,extras)
 
+def xenlight_golang_generate_helpers(path = None, types = None, comment = 
None):
+"""
+Generate a .go file (helpers.gen.go by default)
+that contains helper functions for marshaling between
+C and Go types.
+"""
+if path is None:
+path = 'helpers.gen.go'
+
+with open(path, 'w') as f:
+if comment is not None:
+f.write(comment)
+f.write('package xenlight\n')
+
+# Cgo preamble
+f.write('/*\n')
+f.write('#cgo LDFLAGS: -lxenlight\n')
+f.write('#include \n')
+f.write('#include \n')
+f.write('\n')
+
+f.write('*/\nimport "C"\n')
+
+for ty in types:
+if not isinstance(ty, idl.Struct):
+continue
+
+f.write(xenlight_golang_define_from_C(ty))
+f.write('\n')
+
+go_fmt(path)
+
+def xenlight_golang_define_from_C(ty = None):
+"""
+Define the fromC marshaling function for the type
+represented by ty.
+"""
+func = 'func (x *{}) fromC(xc *C.{}) error {{\n {} \n return nil}}\n'
+
+goname = xenlight_golang_fmt_name(ty.typename)
+cname  = ty.typename
+
+body = ''
+
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+# TODO
+continue
+
+body += xenlight_golang_convert_from_C(f)
+
+elif isinstance(f.type, idl.Struct):
+# Go through the fields of the anonymous nested struct.
+for nf in f.type.fields:
+body += xenlight_golang_convert_from_C(nf,outer_name=f.name)
+
+elif isinstance(f.type, idl.KeyedUnion):
+pass
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+return func.format(goname, cname, body)
+
+def xenlight_golang_convert_from_C(ty = None, outer_name = None):
+"""
+Returns a line of Go