Re: [Xen-devel] [PATCH 06/24] golang/xenlight: re-name Bitmap marshaling functions

[George Dunlap <george.dunlap@citrix.com>]

On 10/7/19 4:12 PM, Nick Rosbrook wrote:
> From: Nick Rosbrook <rosbrookn@ainfosec.com>
> 
> Re-name and modify signature of toGo function to fromC. The reason for
> using 'fromC' rather than 'toGo' is that it is not a good idea to define
> methods on the C types. Also, add error return type to Bitmap's toC function.
> 
> Finally, as code-cleanup, re-organize the Bitmap type's comments as per
> Go conventions.
> 
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>

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

With one minor comment...


> +func (bm *Bitmap) fromC(cbm *C.libxl_bitmap) error {
> +	// Alloc a Go slice for the bytes
> +	size := int(cbm.size)
> +	bm.bitmap = make([]C.uint8_t, size)
> +
> +	// Make a slice pointing to the C array
> +	mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
> +
> +	// And copy the C array into the Go array
> +	copy(bm.bitmap, mapslice)
> +
> +	return nil
> +}
> +
> +func (bm *Bitmap) toC() (C.libxl_bitmap, error) {
> +	var cbm C.libxl_bitmap
> +
> +	size := len(bm.bitmap)
> +	cbm.size = C.uint32_t(size)
> +	cbm._map = (*C.uint8_t)(C.malloc(C.ulong(cbm.size) * C.sizeof_uint8_t))
> +	cslice := (*[1 << 31]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]

Any particular reason to use `cslice` here rather than `mapslice` (or
vice versa)?

Not a big deal, but since they're of the came element in the C struct,
it seems like it would be better to give them the same name.  (Don't
have a strong opinion on which one).

 -George

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