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é <berrange@xxxxxxxxxx>
---
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
