On Tue, Aug 08, 2023 at 01:15:16PM +0100, Richard W.M. Jones wrote: > On Tue, Aug 08, 2023 at 02:36:12PM +0300, Nir Soffer wrote: > > On Thu, Aug 3, 2023 at 4:57 AM Eric Blake <ebl...@redhat.com> wrote: > > > func copy_uint32_array(entries *C.uint32_t, count C.size_t) []uint32 { > > > + if (uint64(count) > 64*1024*1024) { > > > + panic(\"violation of state machine guarantee\") > > > > This is unwanted in a library, it means the entire application will crash > > because of a bug in the library. Can we convert this to an error in the > > caller? > > In Eric's defence I want to point out that this is (supposed to be) a > "should never happen" internal error, like an assert in C. What's the > proper way to handle an internal error?
Indeed - the intent here is that there is no error to return to the caller, because the caller cannot ever set up any condition where this will fail. > > > > + } > > > 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] > > > > Can we require Go 1.17? (current version is 1.20) > > > > In Go >= 1.17, we can use something like: > > > > s := unsafe.Slice(C.uint32_t, length) > > Golang 1.17 was released in Aug 2021 (2 years ago) which is still > fairly recent. > > On the other hand, RHEL 8 has 1.19 for some reason -- RHEL 8 seem to > rebasing golang like crazy. > > So I guess this would be fine. > > > > - copy(ret, s) > > > + addr := uintptr(unsafe.Pointer(entries)) > > > + for i := 0; i < int(count); i++ { > > > + ptr := (*C.uint32_t)(unsafe.Pointer(addr)) > > > + ret[i] = uint32(*ptr) > > > + addr += unsafe.Sizeof(*ptr) > > > + } > > > > This loop is worse than the ugly line creating a slice. > > With a slice we can do: > > > > for i, item := range s { > > ret[i] = uint32(item) > > } The real test is whether we can also write nice slice semantics for the loop in 6/25, when converting an array of C structs to a Go array. If requiring Go > 1.17 is the way to do that, then I can experiment with requiring that as a minimum during configure. -- 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