Stop mixing Go allocated memory with C operations for typed parameters
and exclusively rely on C allocated memory. This greatly reduces the
number of type casts giving clearer code.

Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
---
 domain.go              |  28 ++++-----
 typedparams.go         |  87 ++++++++++++++------------
 typedparams_test.go    |   8 ++-
 typedparams_wrapper.go | 139 +++++++++++++++++++++++++++++++++++++++++
 typedparams_wrapper.h  |  82 ++++++++++++++++++++++++
 5 files changed, 285 insertions(+), 59 deletions(-)
 create mode 100644 typedparams_wrapper.go
 create mode 100644 typedparams_wrapper.h

diff --git a/domain.go b/domain.go
index b39de47..8f7f030 100644
--- a/domain.go
+++ b/domain.go
@@ -2104,16 +2104,15 @@ func (d *Domain) BlockCopy(disk string, destxml string, 
params *DomainBlockCopyP
 
        info := getBlockCopyParameterFieldInfo(params)
 
-       cparams, gerr := typedParamsPackNew(info)
+       cparams, cnparams, gerr := typedParamsPackNew(info)
        if gerr != nil {
                return gerr
        }
-       nparams := len(*cparams)
 
-       defer 
C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), 
C.int(nparams))
+       defer C.virTypedParamsFree(cparams, cnparams)
 
        var err C.virError
-       ret := C.virDomainBlockCopyWrapper(d.ptr, cdisk, cdestxml, 
(*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), C.int(nparams), 
C.uint(flags), &err)
+       ret := C.virDomainBlockCopyWrapper(d.ptr, cdisk, cdestxml, cparams, 
cnparams, C.uint(flags), &err)
        if ret == -1 {
                return makeError(&err)
        }
@@ -2375,16 +2374,15 @@ func getMigrateParameterFieldInfo(params 
*DomainMigrateParameters) map[string]ty
 func (d *Domain) Migrate3(dconn *Connect, params *DomainMigrateParameters, 
flags DomainMigrateFlags) (*Domain, error) {
 
        info := getMigrateParameterFieldInfo(params)
-       cparams, gerr := typedParamsPackNew(info)
+       cparams, cnparams, gerr := typedParamsPackNew(info)
        if gerr != nil {
                return nil, gerr
        }
-       nparams := len(*cparams)
 
-       defer 
C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), 
C.int(nparams))
+       defer C.virTypedParamsFree(cparams, cnparams)
 
        var err C.virError
-       ret := C.virDomainMigrate3Wrapper(d.ptr, dconn.ptr, 
(*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), C.uint(nparams), 
C.uint(flags), &err)
+       ret := C.virDomainMigrate3Wrapper(d.ptr, dconn.ptr, cparams, 
C.uint(cnparams), C.uint(flags), &err)
        if ret == nil {
                return nil, makeError(&err)
        }
@@ -2455,16 +2453,15 @@ func (d *Domain) MigrateToURI3(dconnuri string, params 
*DomainMigrateParameters,
        }
 
        info := getMigrateParameterFieldInfo(params)
-       cparams, gerr := typedParamsPackNew(info)
+       cparams, cnparams, gerr := typedParamsPackNew(info)
        if gerr != nil {
                return gerr
        }
-       nparams := len(*cparams)
 
-       defer 
C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), 
C.int(nparams))
+       defer C.virTypedParamsFree(cparams, cnparams)
 
        var err C.virError
-       ret := C.virDomainMigrateToURI3Wrapper(d.ptr, cdconnuri, 
(*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), C.uint(nparams), 
C.uint(flags), &err)
+       ret := C.virDomainMigrateToURI3Wrapper(d.ptr, cdconnuri, cparams, 
C.uint(cnparams), C.uint(flags), &err)
        if ret == -1 {
                return makeError(&err)
        }
@@ -4272,16 +4269,15 @@ func (d *Domain) SetIOThreadParams(iothreadid uint, 
params *DomainSetIOThreadPar
        }
        info := getSetIOThreadParamsFieldInfo(params)
 
-       cparams, gerr := typedParamsPackNew(info)
+       cparams, cnparams, gerr := typedParamsPackNew(info)
        if gerr != nil {
                return gerr
        }
-       nparams := len(*cparams)
 
-       defer 
C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), 
C.int(nparams))
+       defer C.virTypedParamsFree(cparams, cnparams)
 
        var err C.virError
-       ret := C.virDomainSetIOThreadParamsWrapper(d.ptr, C.uint(iothreadid), 
(*C.virTypedParameter)(unsafe.Pointer(&(*cparams)[0])), C.int(nparams), 
C.uint(flags), &err)
+       ret := C.virDomainSetIOThreadParamsWrapper(d.ptr, C.uint(iothreadid), 
cparams, cnparams, C.uint(flags), &err)
        if ret == -1 {
                return makeError(&err)
        }
diff --git a/typedparams.go b/typedparams.go
index e8ceae0..a1bdffd 100644
--- a/typedparams.go
+++ b/typedparams.go
@@ -32,6 +32,7 @@ package libvirt
 #include <libvirt/virterror.h>
 #include <stdlib.h>
 #include <string.h>
+#include "typedparams_wrapper.h"
 */
 import "C"
 
@@ -197,66 +198,70 @@ func typedParamsPack(cparams []C.virTypedParameter, 
infomap map[string]typedPara
        return typedParamsPackLen(&cparams[0], len(cparams), infomap)
 }
 
-func typedParamsPackNew(infomap map[string]typedParamsFieldInfo) 
(*[]C.virTypedParameter, error) {
-       nparams := 0
-       for _, value := range infomap {
-               if !*value.set {
-                       continue
-               }
+func typedParamsPackNew(infomap map[string]typedParamsFieldInfo) 
(*C.virTypedParameter, C.int, error) {
+       var cparams C.virTypedParameterPtr
+       var nparams C.int
+       var maxparams C.int
 
-               if value.sl != nil {
-                       nparams += len(*value.sl)
-               } else {
-                       nparams++
-               }
-       }
+       defer C.virTypedParamsFree(cparams, nparams)
 
-       cparams := make([]C.virTypedParameter, nparams)
-       nparams = 0
-       for key, value := range infomap {
+       for name, value := range infomap {
                if !*value.set {
                        continue
                }
 
-               cfield := C.CString(key)
-               defer C.free(unsafe.Pointer(cfield))
-               clen := len(key) + 1
-               if clen > C.VIR_TYPED_PARAM_FIELD_LENGTH {
-                       clen = C.VIR_TYPED_PARAM_FIELD_LENGTH
-               }
+               cname := C.CString(name)
+               defer C.free(unsafe.Pointer(cname))
                if value.sl != nil {
+                       /* We're not actually using 
virTypedParamsAddStringList, as it is
+                        * easier to avoid creating a 'char **' in Go to hold 
all the strings.
+                        * We none the less do a version check, because earlier 
libvirts
+                        * would not expect to see multiple string values in a 
typed params
+                        * list with the same field name
+                        */
+                       if C.LIBVIR_VERSION_NUMBER < 1002017 {
+                               return nil, 0, 
makeNotImplementedError("virTypedParamsAddStringList")
+                       }
                        for i := 0; i < len(*value.sl); i++ {
-                               cparam := &cparams[nparams]
-                               cparam._type = C.VIR_TYPED_PARAM_STRING
-                               C.memcpy(unsafe.Pointer(&cparam.field[0]), 
unsafe.Pointer(cfield), C.size_t(clen))
-                               nparams++
+                               cvalue := C.CString((*value.sl)[i])
+                               defer C.free(unsafe.Pointer(cvalue))
+                               var err C.virError
+                               ret := 
C.virTypedParamsAddStringWrapper(&cparams, &nparams, &maxparams, cname, cvalue, 
&err)
+                               if ret < 0 {
+                                       return nil, 0, makeError(&err)
+                               }
                        }
                } else {
-                       cparam := &cparams[nparams]
+                       var err C.virError
+                       var ret C.int
                        if value.i != nil {
-                               cparam._type = C.VIR_TYPED_PARAM_INT
+                               ret = C.virTypedParamsAddIntWrapper(&cparams, 
&nparams, &maxparams, cname, C.int(*value.i), &err)
                        } else if value.ui != nil {
-                               cparam._type = C.VIR_TYPED_PARAM_UINT
+                               ret = C.virTypedParamsAddUIntWrapper(&cparams, 
&nparams, &maxparams, cname, C.uint(*value.ui), &err)
                        } else if value.l != nil {
-                               cparam._type = C.VIR_TYPED_PARAM_LLONG
+                               ret = C.virTypedParamsAddLLongWrapper(&cparams, 
&nparams, &maxparams, cname, C.longlong(*value.l), &err)
                        } else if value.ul != nil {
-                               cparam._type = C.VIR_TYPED_PARAM_ULLONG
+                               ret = 
C.virTypedParamsAddULLongWrapper(&cparams, &nparams, &maxparams, cname, 
C.ulonglong(*value.ul), &err)
                        } else if value.b != nil {
-                               cparam._type = C.VIR_TYPED_PARAM_BOOLEAN
+                               v := 0
+                               if *value.b {
+                                       v = 1
+                               }
+                               ret = 
C.virTypedParamsAddBooleanWrapper(&cparams, &nparams, &maxparams, cname, 
C.int(v), &err)
                        } else if value.d != nil {
-                               cparam._type = C.VIR_TYPED_PARAM_DOUBLE
+                               ret = 
C.virTypedParamsAddDoubleWrapper(&cparams, &nparams, &maxparams, cname, 
C.double(*value.i), &err)
                        } else if value.s != nil {
-                               cparam._type = C.VIR_TYPED_PARAM_STRING
+                               cvalue := C.CString(*value.s)
+                               defer C.free(unsafe.Pointer(cvalue))
+                               ret = 
C.virTypedParamsAddStringWrapper(&cparams, &nparams, &maxparams, cname, cvalue, 
&err)
+                       } else {
+                               return nil, 0, fmt.Errorf("No typed parameter 
value set for field '%s'", name)
+                       }
+                       if ret < 0 {
+                               return nil, 0, makeError(&err)
                        }
-                       C.memcpy(unsafe.Pointer(&cparam.field[0]), 
unsafe.Pointer(cfield), C.size_t(clen))
-                       nparams++
                }
        }
 
-       err := typedParamsPack(cparams, infomap)
-       if err != nil {
-               
C.virTypedParamsClear((*C.virTypedParameter)(unsafe.Pointer(&cparams[0])), 
C.int(nparams))
-               return nil, err
-       }
-       return &cparams, nil
+       return cparams, nparams, nil
 }
diff --git a/typedparams_test.go b/typedparams_test.go
index 8bc980a..ff3783b 100644
--- a/typedparams_test.go
+++ b/typedparams_test.go
@@ -77,12 +77,16 @@ func TestPackUnpack(t *testing.T) {
                sl:  &got3,
        }
 
-       params, err := typedParamsPackNew(infoin)
+       params, nparams, err := typedParamsPackNew(infoin)
        if err != nil {
+               lverr, ok := err.(Error)
+               if ok && lverr.Code == ERR_NO_SUPPORT {
+                       return
+               }
                t.Fatal(err)
        }
 
-       nout, err := typedParamsUnpack(*params, infoout)
+       nout, err := typedParamsUnpackLen(params, int(nparams), infoout)
        if err != nil {
                t.Fatal(err)
        }
diff --git a/typedparams_wrapper.go b/typedparams_wrapper.go
new file mode 100644
index 0000000..c0248ce
--- /dev/null
+++ b/typedparams_wrapper.go
@@ -0,0 +1,139 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ */
+
+package libvirt
+
+/*
+#cgo pkg-config: libvirt
+#include <assert.h>
+#include "typedparams_wrapper.h"
+
+int
+virTypedParamsAddIntWrapper(virTypedParameterPtr *params,
+                           int *nparams,
+                           int *maxparams,
+                           const char *name,
+                           int value,
+                           virErrorPtr err)
+{
+    int ret = virTypedParamsAddInt(params, nparams, maxparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsAddUIntWrapper(virTypedParameterPtr *params,
+                            int *nparams,
+                            int *maxparams,
+                            const char *name,
+                            unsigned int value,
+                            virErrorPtr err)
+{
+    int ret = virTypedParamsAddUInt(params, nparams, maxparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsAddLLongWrapper(virTypedParameterPtr *params,
+                             int *nparams,
+                             int *maxparams,
+                             const char *name,
+                             long long value,
+                             virErrorPtr err)
+{
+    int ret = virTypedParamsAddLLong(params, nparams, maxparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsAddULLongWrapper(virTypedParameterPtr *params,
+                              int *nparams,
+                              int *maxparams,
+                              const char *name,
+                              unsigned long long value,
+                              virErrorPtr err)
+{
+    int ret = virTypedParamsAddULLong(params, nparams, maxparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsAddDoubleWrapper(virTypedParameterPtr *params,
+                              int *nparams,
+                              int *maxparams,
+                              const char *name,
+                              double value,
+                              virErrorPtr err)
+{
+    int ret = virTypedParamsAddDouble(params, nparams, maxparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsAddBooleanWrapper(virTypedParameterPtr *params,
+                               int *nparams,
+                               int *maxparams,
+                               const char *name,
+                               int value,
+                               virErrorPtr err)
+{
+    int ret = virTypedParamsAddBoolean(params, nparams, maxparams, name, 
value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsAddStringWrapper(virTypedParameterPtr *params,
+                              int *nparams,
+                              int *maxparams,
+                              const char *name,
+                              const char *value,
+                              virErrorPtr err)
+{
+    int ret = virTypedParamsAddString(params, nparams, maxparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+*/
+import "C"
diff --git a/typedparams_wrapper.h b/typedparams_wrapper.h
new file mode 100644
index 0000000..d2ef7d6
--- /dev/null
+++ b/typedparams_wrapper.h
@@ -0,0 +1,82 @@
+/*
+ * This file is part of the libvirt-go project
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ */
+
+#ifndef LIBVIRT_GO_TYPEDPARAMS_WRAPPER_H__
+#define LIBVIRT_GO_TYPEDPARAMS_WRAPPER_H__
+
+#include <libvirt/libvirt.h>
+#include <libvirt/virterror.h>
+
+int
+virTypedParamsAddIntWrapper(virTypedParameterPtr *params,
+                           int *nparams,
+                           int *maxparams,
+                           const char *name,
+                           int value,
+                           virErrorPtr err);
+int
+virTypedParamsAddUIntWrapper(virTypedParameterPtr *params,
+                            int *nparams,
+                            int *maxparams,
+                            const char *name,
+                            unsigned int value,
+                            virErrorPtr err);
+int
+virTypedParamsAddLLongWrapper(virTypedParameterPtr *params,
+                             int *nparams,
+                             int *maxparams,
+                             const char *name,
+                             long long value,
+                             virErrorPtr err);
+int
+virTypedParamsAddULLongWrapper(virTypedParameterPtr *params,
+                              int *nparams,
+                              int *maxparams,
+                              const char *name,
+                              unsigned long long value,
+                              virErrorPtr err);
+int
+virTypedParamsAddDoubleWrapper(virTypedParameterPtr *params,
+                              int *nparams,
+                              int *maxparams,
+                              const char *name,
+                              double value,
+                              virErrorPtr err);
+int
+virTypedParamsAddBooleanWrapper(virTypedParameterPtr *params,
+                               int *nparams,
+                               int *maxparams,
+                               const char *name,
+                               int value,
+                               virErrorPtr err);
+int
+virTypedParamsAddStringWrapper(virTypedParameterPtr *params,
+                              int *nparams,
+                              int *maxparams,
+                              const char *name,
+                              const char *value,
+                              virErrorPtr err);
+
+#endif /* LIBVIRT_GO_TYPEDPARAMS_WRAPPER_H__ */
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to