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
>
_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs
