| Message ID | 20190628083148.1747-1-nicolas.belouin@gandi.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] golang/xenlight: Fix type issues with recent Go version | expand |
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>
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>
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
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(-)