Re: [Libguestfs] [libnbd PATCH v4 05/25] golang: Change logic of copy_uint32_array
On Tue, Aug 08, 2023 at 02:36:12PM +0300, Nir Soffer wrote:
On Thu, Aug 3, 2023 at 4:57 AM Eric Blake <eblake(a)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?
> + }
> 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)
}
(I did not try to compile this)
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top