[Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation

Laszlo Ersek lersek at redhat.com
Mon Sep 20 15:23:30 UTC 2021


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
> 




More information about the Libguestfs mailing list