Re: [Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation
On Mon, Sep 20, 2021 at 12:03:51PM +0100, Richard W.M. Jones wrote:
On Mon, Sep 20, 2021 at 11:37:02AM +0100, Daniel P. Berrangé wrote:
> What distro / go version do you see this on, as I can't reproduce
> this pointer problem with a standalone demo app ?
For me this started to happen after upgrading to
golang-bin-1.17-2.fc36.x86_64 in Rawhide. It also caused this error:
Hmm, I still cant reproduce the problem that Laszlo is fixing
$ cat str.c
#include <stdio.h>
void foo(char **str) {
for (int i = 0; str[i] != NULL; i++) {
fprintf(stderr, "%d: %s (0x%p)\n", i, str[i], str[i]);
}
}
$ cat str.go
package main
/*
#cgo LDFLAGS: -L/home/berrange/t/lib -lstr
#include <stdlib.h>
extern void foo(char **str);
*/
import "C"
import (
"fmt"
"unsafe"
)
func array_elem(arr **C.char, idx int) **C.char {
return (**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(arr)) +
(unsafe.Sizeof(arr) * uintptr(idx))))
}
func arg_string_list1(xs []string) **C.char {
r := make([]*C.char, 1+len(xs))
for i, x := range xs {
r[i] = C.CString(x)
}
r[len(xs)] = nil
return &r[0]
}
func arg_string_list2(xs []string) **C.char {
var r **C.char
r = (**C.char)(C.malloc(C.size_t(unsafe.Sizeof(*r) * (1 + uintptr(len(xs))))))
for i, x := range xs {
str := array_elem(r, i)
*str = C.CString(x)
}
str := array_elem(r, len(xs))
*str = nil
return r
}
func free_string_list(argv **C.char) {
for i := 0; ; i++ {
str := (**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(argv)) +
(unsafe.Sizeof(*argv) * uintptr(i))))
if *str == nil {
break
}
fmt.Printf("%x\n", *str)
C.free(unsafe.Pointer(*str))
}
}
func bar(str []string) {
cstr1 := arg_string_list1(str)
defer free_string_list(cstr1)
C.foo(cstr1)
cstr2 := arg_string_list2(str)
defer free_string_list(cstr2)
C.foo(cstr2)
}
func main() {
bar([]string{"hello", "world"})
}
My interpretation is that arg_string_list1 impl here should have
raised the error that Laszlo reports, but both impls work fine
$ gcc -fPIC -c -o str.o str.c
$ gcc -shared -o libstr.so str.o
$ go version
go version go1.17 linux/amd64
$ go build -o str str.go
$ LD_LIBRARY_PATH=/home/berrange/t/lib ./str
0: hello (0x0x1d68970)
1: world (0x0x1d68990)
0: hello (0x0x1d689d0)
1: world (0x0x1d689f0)
1d689d0
1d689f0
1d68970
1d68990
Is my test scearnio there representative of what the failing test
case is doing ? Or is perhaps the C function calling back into
the Go code ?
The reason I'm curious is that the current code for arrays here
matches what libvirt-go-module currently uses in some places, so
I'm wondering if that needs fixing too.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|