* [Xen-devel] [PATCH] golang/xenlight: Fix type issues with recent Go version
@ 2019-06-27 7:58 Nicolas Belouin
2019-06-27 16:24 ` George Dunlap
0 siblings, 1 reply; 3+ messages in thread
From: Nicolas Belouin @ 2019-06-27 7:58 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Jackson, Nicolas Belouin, George Dunlap, Wei Liu
Go is doing more type check (even when using CGo), so these incorrect
use of `unsafe.Pointer` as well as the lack of `unsafe.Pointer` for
these calls no longer compile with recent Go versions.
This does *not* break compatibility with older Go version.
---
tools/golang/xenlight/xenlight.go | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
index 53534d047e..e281328d43 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -854,7 +854,7 @@ func (Ctx *Context) Open() (err error) {
}
ret := C.libxl_ctx_alloc(&Ctx.ctx, C.LIBXL_VERSION,
- 0, unsafe.Pointer(Ctx.logger))
+ 0, (*C.struct_xentoollog_logger)(unsafe.Pointer(Ctx.logger)))
if ret != 0 {
err = Error(-ret)
@@ -869,7 +869,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((*C.struct_xentoollog_logger)(unsafe.Pointer(Ctx.logger)))
return
}
@@ -1170,7 +1170,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 +1190,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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Xen-devel] [PATCH] golang/xenlight: Fix type issues with recent Go version
2019-06-27 7:58 [Xen-devel] [PATCH] golang/xenlight: Fix type issues with recent Go version Nicolas Belouin
@ 2019-06-27 16:24 ` George Dunlap
2019-06-28 7:22 ` Nicolas Belouin
0 siblings, 1 reply; 3+ messages in thread
From: George Dunlap @ 2019-06-27 16:24 UTC (permalink / raw)
To: Nicolas Belouin, xen-devel; +Cc: Ian Jackson, Wei Liu
On 6/27/19 8:58 AM, Nicolas Belouin wrote:
> Go is doing more type check (even when using CGo), so these incorrect
> use of `unsafe.Pointer` as well as the lack of `unsafe.Pointer` for
> these calls no longer compile with recent Go versions.
>
> This does *not* break compatibility with older Go version.
Need a SoB here.
Also, I think a slightly more descriptive commit message would be
helpful; something like:
---
Newer versions of Go have become stricter on acceptable pointer
conversions. Specifically, the following two conversions are no longer
allowed:
- unsafe.Pointer being automatically cast to another type
- A pointer type other than unsafe.Pointer being automatically cast to C
void *
Fix this by adding explicit casts where necessary.
---
> ---
> tools/golang/xenlight/xenlight.go | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> index 53534d047e..e281328d43 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -854,7 +854,7 @@ func (Ctx *Context) Open() (err error) {
> }
>
> ret := C.libxl_ctx_alloc(&Ctx.ctx, C.LIBXL_VERSION,
> - 0, unsafe.Pointer(Ctx.logger))
> + 0, (*C.struct_xentoollog_logger)(unsafe.Pointer(Ctx.logger)))
>
> if ret != 0 {
> err = Error(-ret)
> @@ -869,7 +869,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((*C.struct_xentoollog_logger)(unsafe.Pointer(Ctx.logger)))
I'm wondering if a better approach here would be to have Ctx.logger be
type C.xentoollog_logger, and just do the cast from
xentoollog_logger_stdiostream once when creating the logger.
The other two changes look good, thanks.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Xen-devel] [PATCH] golang/xenlight: Fix type issues with recent Go version
2019-06-27 16:24 ` George Dunlap
@ 2019-06-28 7:22 ` Nicolas Belouin
0 siblings, 0 replies; 3+ messages in thread
From: Nicolas Belouin @ 2019-06-28 7:22 UTC (permalink / raw)
To: George Dunlap; +Cc: xen-devel, Ian Jackson, Wei Liu
On 27/06 17:24, George Dunlap wrote:
> On 6/27/19 8:58 AM, Nicolas Belouin wrote:
> > Go is doing more type check (even when using CGo), so these incorrect
> > use of `unsafe.Pointer` as well as the lack of `unsafe.Pointer` for
> > these calls no longer compile with recent Go versions.
> >
> > This does *not* break compatibility with older Go version.
> Need a SoB here.
Indeed I missed that one.
>
> Also, I think a slightly more descriptive commit message would be
> helpful; something like:
>
> ---
> Newer versions of Go have become stricter on acceptable pointer
> conversions. Specifically, the following two conversions are no longer
> allowed:
>
> - unsafe.Pointer being automatically cast to another type
> - A pointer type other than unsafe.Pointer being automatically cast to C
> void *
>
> Fix this by adding explicit casts where necessary.
> ---
This is indeed more understandable, in fact Golang does not accept any
implicit conversion and these working were more likely a bug in CGo. I
will take your suggested commit message as a base for a V2
>
> > ---
> > tools/golang/xenlight/xenlight.go | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/golang/xenlight/xenlight.go b/tools/golang/xenlight/xenlight.go
> > index 53534d047e..e281328d43 100644
> > --- a/tools/golang/xenlight/xenlight.go
> > +++ b/tools/golang/xenlight/xenlight.go
> > @@ -854,7 +854,7 @@ func (Ctx *Context) Open() (err error) {
> > }
> >
> > ret := C.libxl_ctx_alloc(&Ctx.ctx, C.LIBXL_VERSION,
> > - 0, unsafe.Pointer(Ctx.logger))
> > + 0, (*C.struct_xentoollog_logger)(unsafe.Pointer(Ctx.logger)))
> >
> > if ret != 0 {
> > err = Error(-ret)
> > @@ -869,7 +869,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((*C.struct_xentoollog_logger)(unsafe.Pointer(Ctx.logger)))
>
> I'm wondering if a better approach here would be to have Ctx.logger be
> type C.xentoollog_logger, and just do the cast from
> xentoollog_logger_stdiostream once when creating the logger.
This looks like a better approach as Ctx should not expect a specific
xentoollog_logger type (even though there is only one for now)
>
> The other two changes look good, thanks.
>
> -George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-06-28 7:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-27 7:58 [Xen-devel] [PATCH] golang/xenlight: Fix type issues with recent Go version Nicolas Belouin
2019-06-27 16:24 ` George Dunlap
2019-06-28 7:22 ` Nicolas Belouin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).