[Xen-devel] [PATCH v2] golang/xenlight: Fix type issues with recent Go version

Nicolas Belouin posted 1 patch 16 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20190628083148.1747-1-nicolas.belouin@gandi.net
tools/golang/xenlight/xenlight.go | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

[Xen-devel] [PATCH v2] golang/xenlight: Fix type issues with recent Go version

Posted by Nicolas Belouin 16 weeks ago
Newer versions of Go have become stricter on enforcing the no implicit
conversions policy when using CGo.
Specifically, the following two conversions are no longer allowed:

- unsafe.Pointer being automatically cast to any C pointer
- A pointer type other than unsafe.Pointer being automatically cast to C
void *

Fix this by adding explicit casts where necessary.

Signed-off-by: Nicolas Belouin <nicolas.belouin@gandi.net>
---
 tools/golang/xenlight/xenlight.go | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 53534d047e..a2af6f6ef9 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -122,7 +122,7 @@ type Uuid C.libxl_uuid
 
 type Context struct {
 	ctx    *C.libxl_ctx
-	logger *C.xentoollog_logger_stdiostream
+	logger *C.xentoollog_logger
 }
 
 type Hwcap []C.uint32_t
@@ -847,14 +847,15 @@ func (Ctx *Context) Open() (err error) {
 		return
 	}
 
-	Ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
+	Ctx.logger = (*C.xentoollog_logger)(unsafe.Pointer(
+		C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)))
 	if Ctx.logger == nil {
 		err = fmt.Errorf("Cannot open stdiostream")
 		return
 	}
 
 	ret := C.libxl_ctx_alloc(&Ctx.ctx, C.LIBXL_VERSION,
-		0, unsafe.Pointer(Ctx.logger))
+		0, Ctx.logger)
 
 	if ret != 0 {
 		err = Error(-ret)
@@ -869,7 +870,7 @@ func (Ctx *Context) Close() (err error) {
 	if ret != 0 {
 		err = Error(-ret)
 	}
-	C.xtl_logger_destroy(unsafe.Pointer(Ctx.logger))
+	C.xtl_logger_destroy(Ctx.logger)
 	return
 }
 
@@ -1170,7 +1171,7 @@ func (Ctx *Context) ConsoleGetTty(id Domid, consNum int, conType ConsoleType) (p
 		err = Error(-ret)
 		return
 	}
-	defer C.free(cpath)
+	defer C.free(unsafe.Pointer(cpath))
 
 	path = C.GoString(cpath)
 	return
@@ -1190,7 +1191,7 @@ func (Ctx *Context) PrimaryConsoleGetTty(domid uint32) (path string, err error)
 		err = Error(-ret)
 		return
 	}
-	defer C.free(cpath)
+	defer C.free(unsafe.Pointer(cpath))
 
 	path = C.GoString(cpath)
 	return
-- 
2.22.0


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

Re: [Xen-devel] [PATCH v2] golang/xenlight: Fix type issues with recent Go version

Posted by George Dunlap 16 weeks ago
On 6/28/19 9:31 AM, Nicolas Belouin wrote:
> Newer versions of Go have become stricter on enforcing the no implicit
> conversions policy when using CGo.
> Specifically, the following two conversions are no longer allowed:
> 
> - unsafe.Pointer being automatically cast to any C pointer
> - A pointer type other than unsafe.Pointer being automatically cast to C
> void *
> 
> Fix this by adding explicit casts where necessary.

This looks good, except now the commit message is wrong: We're no longer
simply adding casts; we're changing Context.logger from
*C.xentoollog_logger_stdiostream to *C.xentoollog_logger.  That needs to
be mentioned in the commit message.

I think given what you said about automatic pointer conversion
theoretically never being OK, I might say this instead:

---
Theoretically Go has never allowed automatic pointer conversions; but in
practice earlier versions let some conversions slide.  Newer compilers
are more strict, so make sure that all pointers are converted explicitly
the appropriate types.

Change Context.logger's type to *C.xentoollog_logger, as that's the more
generic type, and results in fewer manual pointer conversions.
---

If you're OK with the above I can change it on check-in.

Otherwise:

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

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

Re: [Xen-devel] [PATCH v2] golang/xenlight: Fix type issues with recent Go version

Posted by Nicolas Belouin 16 weeks ago
On 28/06 18:05, George Dunlap wrote:
> On 6/28/19 9:31 AM, Nicolas Belouin wrote:
> > Newer versions of Go have become stricter on enforcing the no implicit
> > conversions policy when using CGo.
> > Specifically, the following two conversions are no longer allowed:
> > 
> > - unsafe.Pointer being automatically cast to any C pointer
> > - A pointer type other than unsafe.Pointer being automatically cast to C
> > void *
> > 
> > Fix this by adding explicit casts where necessary.
> 
> This looks good, except now the commit message is wrong: We're no longer
> simply adding casts; we're changing Context.logger from
> *C.xentoollog_logger_stdiostream to *C.xentoollog_logger.  That needs to
> be mentioned in the commit message.
> 
> I think given what you said about automatic pointer conversion
> theoretically never being OK, I might say this instead:
> 
> ---
> Theoretically Go has never allowed automatic pointer conversions; but in
> practice earlier versions let some conversions slide.  Newer compilers
> are more strict, so make sure that all pointers are converted explicitly
> the appropriate types.
> 
> Change Context.logger's type to *C.xentoollog_logger, as that's the more
> generic type, and results in fewer manual pointer conversions.
> ---
> 
> If you're OK with the above I can change it on check-in.

I'm OK with the change, thanks.

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

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