[Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation
Daniel P. Berrangé
berrange at redhat.com
Mon Sep 20 15:30:36 UTC 2021
On Mon, Sep 20, 2021 at 05:23:30PM +0200, Laszlo Ersek wrote:
> 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!)
Oh, that makes sense, because you have a Go pointer to the storage for
the struct, and then the 'const *const *str' field is initialized with
a Go pointer returned from the arg_string_list().
You're allowed to pass a Go pointer to C via CGo, but the memory that
points to is not allowed to contained further Go pointers. So the struct
fields must strictly use a C pointer.
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 :|
More information about the Libguestfs
mailing list