Go bindings: fix "C array of strings" -- char** -- allocation

The current "arg_string_list" and "free_string_list" implementations go
back to commit b6f01f3260 ("Add Go (language) bindings.", 2013-07-03).
There are two problems with them:

- "free_string_list" doesn't actually free anything,

- at the *first* such g.Internal_test() call site that passes an
  Ostringlist member inside the Optargs argument, namely:

> 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{}
>                                               }
>                 )

  the "golang/run-bindtests" case crashes:

> panic: runtime error: cgo argument has Go pointer to Go pointer
>
> goroutine 1 [running]:
> libguestfs.org/guestfs.(*Guestfs).Internal_test.func7(0xc000018180,
> 0xadfb60, 0xadfb80, 0xc000010048, 0x0, 0x0, 0x0, 0xae3e10, 0xae3e30,
> 0xade3a0, ...)
>         golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0xa9
> libguestfs.org/guestfs.(*Guestfs).Internal_test(0xc000018180, 0x4ee3a5,
> 0x3, 0xc000061be8, 0xc000061af8, 0x0, 0x0, 0xc000061a00, 0x0, 0x0, ...)
>         golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0x3c9
> main.main()
>         golang/bindtests/bindtests.go:77 +0x151e
> exit status 2
> FAIL run-bindtests (exit status: 1)

In Daniel P. Berrangé's words [1],

> 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.

One pattern to solve the problem has been shown on stackoverflow [2].
Thus, rewrite the "arg_string_list" and "free_string_list" functions
almost entirely in C, following that example.

While this approach is not the most idiomatic Go, as a solution exists
without C helper functions [3], it should still be acceptable, at least as
an incremental improvement, for letting "golang/run-bindtests" pass.

[1] https://listman.redhat.com/archives/libguestfs/2021-September/msg00118.html
[2] https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
[3] https://listman.redhat.com/archives/libguestfs/2021-September/msg00106.html

Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210921192939.32468-1-lersek@redhat.com>
Tested-by: "Richard W.M. Jones" <rjones@redhat.com>
Acked-by: "Richard W.M. Jones" <rjones@redhat.com>
This commit is contained in:
Laszlo Ersek
2021-09-21 21:29:39 +02:00
parent 5c68392175
commit e2f8db27d0

View File

@@ -52,6 +52,40 @@ _go_guestfs_create_flags (unsigned flags)
{
return guestfs_create_flags (flags);
}
// Passing a Go pointer to C via CGo is allowed, but the memory that points to
// is not allowed to contain further Go pointers. The helper functions below
// are one way to implement this, although the same can be achieved purely in
// Go as well. See the discussion here:
// <https://listman.redhat.com/archives/libguestfs/2021-September/msg00101.html>.
typedef char *pChar;
pChar *allocStringArray (size_t nmemb)
{
pChar *array;
array = malloc (sizeof (pChar) * (nmemb + 1));
array[nmemb] = NULL;
return array;
}
void storeString (pChar *array, size_t idx, pChar string)
{
array[idx] = string;
}
void freeStringArray (pChar *array)
{
pChar *position;
pChar string;
position = array;
while ((string = *position) != NULL) {
free (string);
position++;
}
free (array);
}
*/
import \"C\"
@@ -148,12 +182,11 @@ func (g *Guestfs) Close () *GuestfsError {
* C strings and golang []string.
*/
func arg_string_list (xs []string) **C.char {
r := make ([]*C.char, 1 + len (xs))
r := C.allocStringArray (C.size_t (len (xs)))
for i, x := range xs {
r[i] = C.CString (x)
C.storeString (r, C.size_t (i), C.CString (x));
}
r[len (xs)] = nil
return &r[0]
return (**C.char) (r)
}
func count_string_list (argv **C.char) int {
@@ -167,11 +200,7 @@ func count_string_list (argv **C.char) int {
}
func free_string_list (argv **C.char) {
for *argv != nil {
//C.free (*argv)
argv = (**C.char) (unsafe.Pointer (uintptr (unsafe.Pointer (argv)) +
unsafe.Sizeof (*argv)))
}
C.freeStringArray ((*C.pChar) (argv))
}
func return_string_list (argv **C.char) []string {