[13/24] golang/xenlight: re-factor Hwcap type implementation
diff mbox series

Message ID 6dbe8bb54d634e7ebe4eb12245255c75ce22fbce.1570456846.git.rosbrookn@ainfosec.com
State Superseded
Headers show
Series
  • generated Go libxl bindings using IDL
Related show

Commit Message

Nick Rosbrook Oct. 7, 2019, 3:13 p.m. UTC
From: Nick Rosbrook <rosbrookn@ainfosec.com>

Re-define Hwcap as [8]uint32, and implement toC function. Also, re-name and
modify signature of toGo function to fromC.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
---
Cc: George Dunlap <george.dunlap@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wl@xen.org>

 tools/golang/xenlight/xenlight.go | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

Comments

George Dunlap Nov. 13, 2019, 5:58 p.m. UTC | #1
On 10/7/19 4:13 PM, Nick Rosbrook wrote:
> From: Nick Rosbrook <rosbrookn@ainfosec.com>
> 
> Re-define Hwcap as [8]uint32, and implement toC function. Also, re-name and
> modify signature of toGo function to fromC.
> 
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>

Looks good.  Just one comment...

> ---
> Cc: George Dunlap <george.dunlap@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wl@xen.org>
> 
>  tools/golang/xenlight/xenlight.go | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index 3e3753f92e..8d520bbd98 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -307,20 +307,29 @@ type Context struct {
>  	logger *C.xentoollog_logger_stdiostream
>  }
>  
> -type Hwcap []C.uint32_t
> -
> -func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
> -	// Alloc a Go slice for the bytes
> -	size := 8
> -	ghwcap = make([]C.uint32_t, size)
> +// Hwcap represents a libxl_hwcap.
> +type Hwcap [8]uint32
>  
> +func (hwcap *Hwcap) fromC(chwcap *C.libxl_hwcap) error {
>  	// Make a slice pointing to the C array
> -	mapslice := (*[1 << 30]C.uint32_t)(unsafe.Pointer(&chwcap[0]))[:size:size]
> +	mapslice := (*[8]C.uint32_t)(unsafe.Pointer(chwcap))

To be pedantic, this isn't a slice anymore. :-)  Do you want to change
this to 'b' (or maybe 'u' or something like that) and fix the comment?

With that changed:

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

Patch
diff mbox series

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 3e3753f92e..8d520bbd98 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -307,20 +307,29 @@  type Context struct {
 	logger *C.xentoollog_logger_stdiostream
 }
 
-type Hwcap []C.uint32_t
-
-func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
-	// Alloc a Go slice for the bytes
-	size := 8
-	ghwcap = make([]C.uint32_t, size)
+// Hwcap represents a libxl_hwcap.
+type Hwcap [8]uint32
 
+func (hwcap *Hwcap) fromC(chwcap *C.libxl_hwcap) error {
 	// Make a slice pointing to the C array
-	mapslice := (*[1 << 30]C.uint32_t)(unsafe.Pointer(&chwcap[0]))[:size:size]
+	mapslice := (*[8]C.uint32_t)(unsafe.Pointer(chwcap))
 
 	// And copy the C array into the Go array
-	copy(ghwcap, mapslice)
+	for i, v := range mapslice {
+		hwcap[i] = uint32(v)
+	}
 
-	return
+	return nil
+}
+
+func (hwcap *Hwcap) toC() (C.libxl_hwcap, error) {
+	var chwcap C.libxl_hwcap
+
+	for i, v := range hwcap {
+		chwcap[i] = C.uint32_t(v)
+	}
+
+	return chwcap, nil
 }
 
 // KeyValueList represents a libxl_key_value_list.
@@ -465,7 +474,7 @@  func (cphys *C.libxl_physinfo) toGo() (physinfo *Physinfo) {
 	physinfo.SharingFreedPages = uint64(cphys.sharing_freed_pages)
 	physinfo.SharingUsedFrames = uint64(cphys.sharing_used_frames)
 	physinfo.NrNodes = uint32(cphys.nr_nodes)
-	physinfo.HwCap = cphys.hw_cap.toGo()
+	physinfo.HwCap.fromC(&cphys.hw_cap)
 	physinfo.CapHvm = bool(cphys.cap_hvm)
 	physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)