On Sun, Aug 13, 2023 at 08:38:23PM +0300, Nir Soffer wrote:
> On Sat, Aug 12, 2023 at 12:18 AM Eric Blake <ebl...@redhat.com> wrote:
>
> > Go 1.17 or newer is required to use unsafe.Slice(), which in turn
> > allows us to write a simpler conversion from a C array to a Go object
> > during callbacks.
> >
> > +++ b/generator/GoLang.ml
> > @@ -517,10 +517,10 @@ let
> >
> > func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 {
> > ret := make([]uint32, int(count))
> > - // See
> > https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
> > - // TODO: Use unsafe.Slice() when we require Go 1.17.
> > - s := (*[1 << 30]uint32)(unsafe.Pointer(entries))[:count:count]
> >
>
> We have another instance of this pattern in AioBuffer.Slice
Will update that one as well. In fact, I'll probably split this into
two patches: one to bump the minimum, the next to take advantage of
the bump.
>
>
> > - copy(ret, s)
> > + s := unsafe.Slice(entries, count)
> > + for i, item := range s {
> > + ret[i] = uint32(item)
> > + }
> > return ret
> > }
> > ";
> > diff --git a/README.md b/README.md
> > index c7166613..8524038e 100644
> > --- a/README.md
> > +++ b/README.md
> > @@ -105,7 +105,7 @@ ## Building from source
> > * Python >= 3.3 to build the Python 3 bindings and NBD shell (nbdsh).
> > * FUSE 3 to build the nbdfuse program.
> > * Linux >= 6.0 and ublksrv library to build nbdublk program.
> > -* go and cgo, for compiling the golang bindings and tests.
> > +* go and cgo >= 1.17, for compiling the golang bindings and tests.
> > * bash-completion >= 1.99 for tab completion.
> >
> > Optional, only needed to run the test suite:
> > diff --git a/golang/configure/go.mod b/golang/configure/go.mod
> > index ce3e4f39..fcdb28db 100644
> > --- a/golang/configure/go.mod
> > +++ b/golang/configure/go.mod
> > @@ -1,4 +1,4 @@
> > module libguestfs.org/configure
> >
> > -// First version of golang with working module support.
> > -go 1.13
> > +// First version of golang with working module support and unsafe.Slice.
> >
>
> "First version of golang with working module support" is not relevant for
> 1.17,
> maybe "For unsafe.Slice"?
Sure. Given my background with Autoconf, I'm so used to feature-based
testing that it's hard for me to rely on version-based testing; so I
documented both sets of features ("working module support" and
"unsafe.Slice")...
>
>
> > +go 1.17
> > diff --git a/golang/configure/test.go b/golang/configure/test.go
> > index fe742f2b..a15c9ea3 100644
> > --- a/golang/configure/test.go
> > +++ b/golang/configure/test.go
> > @@ -25,8 +25,19 @@
> > import (
> > "fmt"
> > "runtime"
> > + "unsafe"
> > )
> >
> > +func check_slice(arr *uint32, cnt int) []uint32 {
> > + /* We require unsafe.Slice(), introduced in 1.17 */
> > + ret := make([]uint32, cnt)
> > + s := unsafe.Slice(arr, cnt)
> > + for i, item := range s {
> > + ret[i] = uint32(item)
> > + }
> > + return ret
> > +}
> >
>
> I'm not sure what is the purpose of this test - requiring the Go version is
> good
> enough since the code will not compile with an older version. EVen if it
> would,
> it will not compile without unsafe.Slice so no special check is needed.
...by adding a witness feature test into the configure probe to
enforce that (to be honest, I wrote the feature test first, but it
failed to compile despite me having a new enough Go; config.log then
contained a Go compiler error that I had not yet bumped the version
line in go.mod as the reason). But you are correct in observing that
unsafe.Slice won't compile without tweaking the go.mod minimum version
line, and it appears that the Go community is more reliant on version
numbers than feature tests. While I continue to wonder if the Go
community is baking itself into problems down the road by relying too
heavily on versions rather than feature tests (you can't backport
features across version numbers and then still take advantage of
them), I also acknowledge that the Go community does not have the
fractured baggage of the C community built up over years of divergent
Unix strains, which really did make feature tests the ideal solution.
In short, dropping the feature test in the configure portion is an
acceptable compromise given that a version test is already mandatory,
so I'll do that.
Now in as commits 41e921b5..f2dac110
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
