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

The current "arg_string_list" and "free_string_list" implementations go back to commit b6f01f32606d ("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: the "golang/run-bindtests" case crashes: In Daniel P. Berrangé's words [1], 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-err... [3] https://listman.redhat.com/archives/libguestfs/2021-September/msg00106.html Cc: "Daniel P. Berrangé" <berrange(a)redhat.com&gt; Cc: "Richard W.M. Jones" <rjones(a)redhat.com&gt; Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: v2: - update commit message [Daniel, Rich] - update code comment [Daniel, Rich] - no code changes - retested generator/golang.ml | 47 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/generator/golang.ml b/generator/golang.ml index d328abe4ed08..0d6a92367f9b 100644 --- a/generator/golang.ml +++ b/generator/golang.ml @@ -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.ht...;. +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 { -- 2.19.1.3.g30247aa5d201