10:23 a.m.
On 09/20/21 14:33, Daniel P. Berrangé wrote:
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
Can you create a new structure type, make the C function take the structure (or a pointer
to the structure), and in the structure, make the field have this type:
char * const * str;
Because this is the scenario where the libguestfs test suite fails (panics). The
libguestfs test suite has a *different* case that does match your example directly, and
*that* case works in the libguestfs test suite flawlessly. The panic surfaces only in the
"char*const* embedded in struct" case. (I assume "const" makes no
difference, but who knows!)
$ 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 ?
No, your case represents the one that works fine in libguestfs too. From
"golang/bindtests/bindtests.go", generated at commit f47e0bb67254:
41 if err := g.Internal_test ("abc", nil, []string{}, false, 0, 0,
"123", "456", []byte{'a', 'b', 'c',
'\000', 'a', 'b', 'c'},
&guestfs.OptargsInternal_test{Oint64_is_set: true, Oint64: 1, Ostring_is_set: true,
Ostring: "string"}); err != nil {
The third argument is such a stringlist, and it works OK.
But this one panics due to "Ostringlist":
77 if err := g.Internal_test ("abc", string_addr ("def"),
[]string{}, false, 0, 0, "123", "456", []byte{'a',
'b', 'c', '\000', 'a', 'b', 'c'},
&guestfs.OptargsInternal_test{Ostringlist_is_set: true, Ostringlist: []string{}}); err
!= nil {
The callee (Internal_test()) is in the also-generated source file
"golang/src/libguestfs.org/guestfs/guestfs.go".
Thanks!
Laszlo
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