[go PATCH 3/8] Change typedParamsPackNew to use a C allocated array

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux