Stop playing games accessing struct fields directly from go and call the
supported libvirt APIs instead. This makes the code clearer and safer
from Go.

The string list code still needs direct pointer games, since libvirt
does not expose any virTypedParamsGetStringList() method, as none of its
APIs actually require this feature yet. The Go binding still wants this
impl at least for the test suite in order to validate the packing of
string lists.

Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
---
 connect.go             |  18 +++---
 domain.go              |   8 +--
 domain_events.go       |  10 ++--
 typedparams.go         | 128 ++++++++++++++++++++++-------------------
 typedparams_test.go    |   2 +-
 typedparams_wrapper.go | 101 ++++++++++++++++++++++++++++++++
 typedparams_wrapper.h  |  44 ++++++++++++++
 7 files changed, 233 insertions(+), 78 deletions(-)

diff --git a/connect.go b/connect.go
index 5044def..e2ff88a 100644
--- a/connect.go
+++ b/connect.go
@@ -2811,7 +2811,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                state := &DomainStatsState{}
                stateInfo := getDomainStatsStateFieldInfo(state)
 
-               count, gerr := typedParamsUnpackLen(cdomstats.params, 
int(cdomstats.nparams), stateInfo)
+               count, gerr := typedParamsUnpackLen(cdomstats.params, 
cdomstats.nparams, stateInfo)
                if gerr != nil {
                        return []DomainStats{}, gerr
                }
@@ -2822,7 +2822,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                cpu := &DomainStatsCPU{}
                cpuInfo := getDomainStatsCPUFieldInfo(cpu)
 
-               count, gerr = typedParamsUnpackLen(cdomstats.params, 
int(cdomstats.nparams), cpuInfo)
+               count, gerr = typedParamsUnpackLen(cdomstats.params, 
cdomstats.nparams, cpuInfo)
                if gerr != nil {
                        return []DomainStats{}, gerr
                }
@@ -2833,7 +2833,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                balloon := &DomainStatsBalloon{}
                balloonInfo := getDomainStatsBalloonFieldInfo(balloon)
 
-               count, gerr = typedParamsUnpackLen(cdomstats.params, 
int(cdomstats.nparams), balloonInfo)
+               count, gerr = typedParamsUnpackLen(cdomstats.params, 
cdomstats.nparams, balloonInfo)
                if gerr != nil {
                        return []DomainStats{}, gerr
                }
@@ -2844,7 +2844,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                perf := &DomainStatsPerf{}
                perfInfo := getDomainStatsPerfFieldInfo(perf)
 
-               count, gerr = typedParamsUnpackLen(cdomstats.params, 
int(cdomstats.nparams), perfInfo)
+               count, gerr = typedParamsUnpackLen(cdomstats.params, 
cdomstats.nparams, perfInfo)
                if gerr != nil {
                        return []DomainStats{}, gerr
                }
@@ -2855,7 +2855,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                lengths := domainStatsLengths{}
                lengthsInfo := getDomainStatsLengthsFieldInfo(&lengths)
 
-               count, gerr = typedParamsUnpackLen(cdomstats.params, 
int(cdomstats.nparams), lengthsInfo)
+               count, gerr = typedParamsUnpackLen(cdomstats.params, 
cdomstats.nparams, lengthsInfo)
                if gerr != nil {
                        return []DomainStats{}, gerr
                }
@@ -2871,7 +2871,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                                vcpu := DomainStatsVcpu{}
                                vcpuInfo := getDomainStatsVcpuFieldInfo(j, 
&vcpu)
 
-                               count, gerr = 
typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), vcpuInfo)
+                               count, gerr = 
typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, vcpuInfo)
                                if gerr != nil {
                                        return []DomainStats{}, gerr
                                }
@@ -2889,7 +2889,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                                block := DomainStatsBlock{}
                                blockInfo := getDomainStatsBlockFieldInfo(j, 
&block)
 
-                               count, gerr = 
typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), blockInfo)
+                               count, gerr = 
typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, blockInfo)
                                if gerr != nil {
                                        return []DomainStats{}, gerr
                                }
@@ -2905,7 +2905,7 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, 
statsTypes DomainStatsTypes,
                                net := DomainStatsNet{}
                                netInfo := getDomainStatsNetFieldInfo(j, &net)
 
-                               count, gerr = 
typedParamsUnpackLen(cdomstats.params, int(cdomstats.nparams), netInfo)
+                               count, gerr = 
typedParamsUnpackLen(cdomstats.params, cdomstats.nparams, netInfo)
                                if gerr != nil {
                                        return []DomainStats{}, gerr
                                }
@@ -2977,7 +2977,7 @@ func (c *Connect) GetSEVInfo(flags uint32) 
(*NodeSEVParameters, error) {
 
        defer C.virTypedParamsFree(cparams, nparams)
 
-       _, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+       _, gerr := typedParamsUnpackLen(cparams, nparams, info)
        if gerr != nil {
                return nil, gerr
        }
diff --git a/domain.go b/domain.go
index 8f7f030..09d6a71 100644
--- a/domain.go
+++ b/domain.go
@@ -3207,7 +3207,7 @@ func (d *Domain) GetJobStats(flags 
DomainGetJobStatsFlags) (*DomainJobInfo, erro
        params := DomainJobInfo{}
        info := getDomainJobInfoFieldInfo(&params)
 
-       _, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+       _, gerr := typedParamsUnpackLen(cparams, nparams, info)
        if gerr != nil {
                return nil, gerr
        }
@@ -3585,7 +3585,7 @@ func (d *Domain) GetPerfEvents(flags 
DomainModificationImpact) (*DomainPerfEvent
 
        defer C.virTypedParamsFree(cparams, nparams)
 
-       _, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+       _, gerr := typedParamsUnpackLen(cparams, nparams, info)
        if gerr != nil {
                return nil, gerr
        }
@@ -4644,7 +4644,7 @@ func (d *Domain) GetGuestVcpus(flags uint32) 
(*DomainGuestVcpus, error) {
 
        defer C.virTypedParamsFree(cparams, C.int(nparams))
 
-       _, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+       _, gerr := typedParamsUnpackLen(cparams, C.int(nparams), info)
        if gerr != nil {
                return nil, gerr
        }
@@ -4861,7 +4861,7 @@ func (d *Domain) GetLaunchSecurityInfo(flags uint32) 
(*DomainLaunchSecurityParam
 
        defer C.virTypedParamsFree(cparams, nparams)
 
-       _, gerr := typedParamsUnpackLen(cparams, int(nparams), info)
+       _, gerr := typedParamsUnpackLen(cparams, nparams, info)
        if gerr != nil {
                return nil, gerr
        }
diff --git a/domain_events.go b/domain_events.go
index fe46c5e..b4bff7b 100644
--- a/domain_events.go
+++ b/domain_events.go
@@ -763,7 +763,7 @@ func domainEventTunableGetPin(params 
C.virTypedParameterPtr, nparams C.int) *Dom
        numvcpus, numiothreads := countPinInfo(params, nparams)
        pinInfo := getDomainPinTempFieldInfo(numvcpus, numiothreads, &pin)
 
-       num, err := typedParamsUnpackLen(params, int(nparams), pinInfo)
+       num, err := typedParamsUnpackLen(params, nparams, pinInfo)
        if num == 0 || err != nil {
                return nil
        }
@@ -820,7 +820,7 @@ func domainEventTunableCallback(c C.virConnectPtr, d 
C.virDomainPtr, params C.vi
        var sched DomainSchedulerParameters
        schedInfo := getDomainTuneSchedulerParametersFieldInfo(&sched)
 
-       num, _ := typedParamsUnpackLen(params, int(nparams), schedInfo)
+       num, _ := typedParamsUnpackLen(params, nparams, schedInfo)
        if num > 0 {
                eventDetails.CpuSched = &sched
        }
@@ -831,12 +831,12 @@ func domainEventTunableCallback(c C.virConnectPtr, d 
C.virDomainPtr, params C.vi
                        s:   &eventDetails.BlkdevDisk,
                },
        }
-       typedParamsUnpackLen(params, int(nparams), blknameInfo)
+       typedParamsUnpackLen(params, nparams, blknameInfo)
 
        var blktune DomainBlockIoTuneParameters
        blktuneInfo := getTuneBlockIoTuneParametersFieldInfo(&blktune)
 
-       num, _ = typedParamsUnpackLen(params, int(nparams), blktuneInfo)
+       num, _ = typedParamsUnpackLen(params, nparams, blktuneInfo)
        if num > 0 {
                eventDetails.BlkdevTune = &blktune
        }
@@ -910,7 +910,7 @@ func domainEventJobCompletedCallback(c C.virConnectPtr, d 
C.virDomainPtr, params
        eventDetails := &DomainEventJobCompleted{}
        info := getDomainJobInfoFieldInfo(&eventDetails.Info)
 
-       typedParamsUnpackLen(params, int(nparams), info)
+       typedParamsUnpackLen(params, nparams, info)
 
        callbackFunc := getCallbackId(goCallbackId)
        callback, ok := callbackFunc.(DomainEventJobCompletedCallback)
diff --git a/typedparams.go b/typedparams.go
index a1bdffd..bda785f 100644
--- a/typedparams.go
+++ b/typedparams.go
@@ -53,76 +53,86 @@ type typedParamsFieldInfo struct {
        sl  *[]string
 }
 
-func typedParamsUnpackLen(cparams *C.virTypedParameter, nparams int, infomap 
map[string]typedParamsFieldInfo) (uint, error) {
+func typedParamsUnpackLen(cparams *C.virTypedParameter, cnparams C.int, 
infomap map[string]typedParamsFieldInfo) (uint, error) {
        count := uint(0)
-       for i := 0; i < nparams; i++ {
-               var cparam *C.virTypedParameter
-               cparam = 
(*C.virTypedParameter)(unsafe.Pointer(uintptr(unsafe.Pointer(cparams)) + 
unsafe.Sizeof(*cparam)*uintptr(i)))
-               name := C.GoString((*C.char)(unsafe.Pointer(&cparam.field)))
-               info, ok := infomap[name]
-               if !ok {
-                       // Ignore unknown keys so that we don't break if
-                       // run against a newer libvirt that returns more
-                       // parameters than we currently have code to
-                       // consume
-                       continue
-               }
-               switch cparam._type {
-               case C.VIR_TYPED_PARAM_INT:
-                       if info.i == nil {
-                               return 0, fmt.Errorf("field %s expects an int", 
name)
-                       }
-                       *info.i = int(*(*C.int)(unsafe.Pointer(&cparam.value)))
-                       *info.set = true
-               case C.VIR_TYPED_PARAM_UINT:
-                       if info.ui == nil {
-                               return 0, fmt.Errorf("field %s expects a uint", 
name)
-                       }
-                       *info.ui = 
uint(*(*C.uint)(unsafe.Pointer(&cparam.value)))
-                       *info.set = true
-               case C.VIR_TYPED_PARAM_LLONG:
-                       if info.l == nil {
-                               return 0, fmt.Errorf("field %s expects an 
int64", name)
-                       }
-                       *info.l = 
int64(*(*C.longlong)(unsafe.Pointer(&cparam.value)))
-                       *info.set = true
-               case C.VIR_TYPED_PARAM_ULLONG:
-                       if info.ul == nil {
-                               return 0, fmt.Errorf("field %s expects a 
uint64", name)
-                       }
-                       *info.ul = 
uint64(*(*C.ulonglong)(unsafe.Pointer(&cparam.value)))
-                       *info.set = true
-               case C.VIR_TYPED_PARAM_DOUBLE:
-                       if info.d == nil {
-                               return 0, fmt.Errorf("field %s expects a 
float64", name)
+       for name, value := range infomap {
+               var err C.virError
+               var ret C.int
+               cname := C.CString(name)
+               defer C.free(unsafe.Pointer(cname))
+               if value.sl != nil {
+                       for i := 0; i < int(cnparams); i++ {
+                               var cparam *C.virTypedParameter
+                               cparam = 
(*C.virTypedParameter)(unsafe.Pointer(uintptr(unsafe.Pointer(cparams)) +
+                                       (unsafe.Sizeof(*cparam) * uintptr(i))))
+                               var cs *C.char
+                               ret = C.virTypedParamsGetStringWrapper(cparam, 
1, cname, &cs, &err)
+                               if ret == 1 {
+                                       *value.sl = append(*value.sl, 
C.GoString(cs))
+                                       *value.set = true
+                                       count++
+                               }
                        }
-                       *info.d = 
float64(*(*C.double)(unsafe.Pointer(&cparam.value)))
-                       *info.set = true
-               case C.VIR_TYPED_PARAM_BOOLEAN:
-                       if info.b == nil {
-                               return 0, fmt.Errorf("field %s expects a bool", 
name)
+               } else {
+                       if value.i != nil {
+                               var ci C.int
+                               ret = C.virTypedParamsGetIntWrapper(cparams, 
cnparams, cname, &ci, &err)
+                               if ret == 1 {
+                                       *value.i = int(ci)
+                               }
+                       } else if value.ui != nil {
+                               var cui C.uint
+                               ret = C.virTypedParamsGetUIntWrapper(cparams, 
cnparams, cname, &cui, &err)
+                               if ret == 1 {
+                                       *value.ui = uint(cui)
+                               }
+                       } else if value.l != nil {
+                               var cl C.longlong
+                               ret = C.virTypedParamsGetLLongWrapper(cparams, 
cnparams, cname, &cl, &err)
+                               if ret == 1 {
+                                       *value.l = int64(cl)
+                               }
+                       } else if value.ul != nil {
+                               var cul C.ulonglong
+                               ret = C.virTypedParamsGetULLongWrapper(cparams, 
cnparams, cname, &cul, &err)
+                               if ret == 1 {
+                                       *value.ul = uint64(cul)
+                               }
+                       } else if value.d != nil {
+                               var cd C.double
+                               ret = C.virTypedParamsGetDoubleWrapper(cparams, 
cnparams, cname, &cd, &err)
+                               if ret == 1 {
+                                       *value.d = float64(cd)
+                               }
+                       } else if value.b != nil {
+                               var cb C.int
+                               ret = 
C.virTypedParamsGetBooleanWrapper(cparams, cnparams, cname, &cb, &err)
+                               if ret == 1 {
+                                       if cb == 1 {
+                                               *value.b = true
+                                       } else {
+                                               *value.b = false
+                                       }
+                               }
+                       } else if value.s != nil {
+                               var cs *C.char
+                               ret = C.virTypedParamsGetStringWrapper(cparams, 
cnparams, cname, &cs, &err)
+                               if ret == 1 {
+                                       *value.s = C.GoString(cs)
+                               }
                        }
-                       *info.b = *(*C.char)(unsafe.Pointer(&cparam.value)) == 1
-                       *info.set = true
-               case C.VIR_TYPED_PARAM_STRING:
-                       if info.s != nil {
-                               *info.s = 
C.GoString(*(**C.char)(unsafe.Pointer(&cparam.value)))
-                               *info.set = true
-                       } else if info.sl != nil {
-                               *info.sl = append(*info.sl, 
C.GoString(*(**C.char)(unsafe.Pointer(&cparam.value))))
-                               *info.set = true
-                       } else {
-                               return 0, fmt.Errorf("field %s expects a 
string/string list", name)
+                       if ret == 1 {
+                               *value.set = true
+                               count++
                        }
                }
-               count++
        }
 
        return count, nil
 }
 
 func typedParamsUnpack(cparams []C.virTypedParameter, infomap 
map[string]typedParamsFieldInfo) (uint, error) {
-       return typedParamsUnpackLen(&cparams[0], len(cparams), infomap)
+       return typedParamsUnpackLen(&cparams[0], C.int(len(cparams)), infomap)
 }
 
 func typedParamsPackLen(cparams *C.virTypedParameter, nparams int, infomap 
map[string]typedParamsFieldInfo) error {
diff --git a/typedparams_test.go b/typedparams_test.go
index ff3783b..dca65b2 100644
--- a/typedparams_test.go
+++ b/typedparams_test.go
@@ -86,7 +86,7 @@ func TestPackUnpack(t *testing.T) {
                t.Fatal(err)
        }
 
-       nout, err := typedParamsUnpackLen(params, int(nparams), infoout)
+       nout, err := typedParamsUnpackLen(params, nparams, infoout)
        if err != nil {
                t.Fatal(err)
        }
diff --git a/typedparams_wrapper.go b/typedparams_wrapper.go
index c0248ce..2209d60 100644
--- a/typedparams_wrapper.go
+++ b/typedparams_wrapper.go
@@ -135,5 +135,106 @@ virTypedParamsAddStringWrapper(virTypedParameterPtr 
*params,
     return ret;
 }
 
+
+int
+virTypedParamsGetIntWrapper(virTypedParameterPtr params,
+                           int nparams,
+                           const char *name,
+                           int *value,
+                           virErrorPtr err)
+{
+    int ret = virTypedParamsGetInt(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetUIntWrapper(virTypedParameterPtr params,
+                            int nparams,
+                            const char *name,
+                            unsigned int *value,
+                            virErrorPtr err)
+{
+    int ret = virTypedParamsGetUInt(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetLLongWrapper(virTypedParameterPtr params,
+                             int nparams,
+                             const char *name,
+                             long long *value,
+                             virErrorPtr err)
+{
+    int ret = virTypedParamsGetLLong(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetULLongWrapper(virTypedParameterPtr params,
+                              int nparams,
+                              const char *name,
+                              unsigned long long *value,
+                              virErrorPtr err)
+{
+    int ret = virTypedParamsGetULLong(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetDoubleWrapper(virTypedParameterPtr params,
+                              int nparams,
+                              const char *name,
+                              double *value,
+                              virErrorPtr err)
+{
+    int ret = virTypedParamsGetDouble(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetBooleanWrapper(virTypedParameterPtr params,
+                               int nparams,
+                               const char *name,
+                               int *value,
+                               virErrorPtr err)
+{
+    int ret = virTypedParamsGetBoolean(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+int
+virTypedParamsGetStringWrapper(virTypedParameterPtr params,
+                              int nparams,
+                              const char *name,
+                              const char **value,
+                              virErrorPtr err)
+{
+    int ret = virTypedParamsGetString(params, nparams, name, value);
+    if (ret < 0) {
+        virCopyLastError(err);
+    }
+    return ret;
+}
+
+
+
 */
 import "C"
diff --git a/typedparams_wrapper.h b/typedparams_wrapper.h
index d2ef7d6..ee626b1 100644
--- a/typedparams_wrapper.h
+++ b/typedparams_wrapper.h
@@ -79,4 +79,48 @@ virTypedParamsAddStringWrapper(virTypedParameterPtr *params,
                               const char *value,
                               virErrorPtr err);
 
+int
+virTypedParamsGetIntWrapper(virTypedParameterPtr params,
+                           int nparams,
+                           const char *name,
+                           int *value,
+                           virErrorPtr err);
+int
+virTypedParamsGetUIntWrapper(virTypedParameterPtr params,
+                            int nparams,
+                            const char *name,
+                            unsigned int *value,
+                            virErrorPtr err);
+int
+virTypedParamsGetLLongWrapper(virTypedParameterPtr params,
+                             int nparams,
+                             const char *name,
+                             long long *value,
+                             virErrorPtr err);
+int
+virTypedParamsGetULLongWrapper(virTypedParameterPtr params,
+                              int nparams,
+                              const char *name,
+                              unsigned long long *value,
+                              virErrorPtr err);
+int
+virTypedParamsGetDoubleWrapper(virTypedParameterPtr params,
+                              int nparams,
+                              const char *name,
+                              double *value,
+                              virErrorPtr err);
+int
+virTypedParamsGetBooleanWrapper(virTypedParameterPtr params,
+                               int nparams,
+                               const char *name,
+                               int *value,
+                               virErrorPtr err);
+int
+virTypedParamsGetStringWrapper(virTypedParameterPtr params,
+                              int nparams,
+                              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