virconf: Make virConfSetValue() clear consumed pointer

The way that virConfSetValue() works (and the way it is even
documented) is that the @value pointer is always consumed.
However, since the first order pointer is passed it leaves
callers in a pickle situation - they always have to set pointer
to NULL after calling virConfSetValue() to avoid touching it.

Let's switch @value to a double pointer and clear it inside the
function.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index f36e269aab6a5e222114d98477fd07eb958081fb..2d363ac2fb33ad5ac29ed761708846190457118b 100644 (file)
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -259,7 +259,7 @@ xenConfigSetInt(virConf *conf, const char *setting, long long l)
     value->next = NULL;
     value->l = l;
 
-    return virConfSetValue(conf, setting, value);
+    return virConfSetValue(conf, setting, &value);
 }
 
 
@@ -274,7 +274,7 @@ xenConfigSetString(virConf *conf, const char *setting, const char *str)
     value->next = NULL;
     value->str = g_strdup(str);
 
-    return virConfSetValue(conf, setting, value);
+    return virConfSetValue(conf, setting, &value);
 }
 
 
@@ -1788,12 +1788,9 @@ xenFormatPCI(virConf *conf, virDomainDef *def)
         }
     }
 
-    if (pciVal->list != NULL) {
-        int ret = virConfSetValue(conf, "pci", pciVal);
-        pciVal = NULL;
-        if (ret < 0)
-            return -1;
-    }
+    if (pciVal->list != NULL &&
+        virConfSetValue(conf, "pci", &pciVal) < 0)
+        return -1;
 
     return 0;
 }
@@ -2000,13 +1997,9 @@ xenFormatCharDev(virConf *conf, virDomainDef *def,
                     }
                 }
 
-                if (serialVal->list != NULL) {
-                    int ret = virConfSetValue(conf, "serial", serialVal);
-
-                    serialVal = NULL;
-                    if (ret < 0)
-                        return -1;
-                }
+                if (serialVal->list != NULL &&
+                    virConfSetValue(conf, "serial", &serialVal) < 0)
+                    return -1;
             }
         } else {
             if (xenConfigSetString(conf, "serial", "none") < 0)
@@ -2264,11 +2257,8 @@ xenFormatVfb(virConf *conf, virDomainDef *def)
             vfb->type = VIR_CONF_LIST;
             vfb->list = g_steal_pointer(&disp);
 
-            if (virConfSetValue(conf, "vfb", vfb) < 0) {
-                vfb = NULL;
+            if (virConfSetValue(conf, "vfb", &vfb) < 0)
                 return -1;
-            }
-            vfb = NULL;
         }
     }
 
@@ -2326,12 +2316,9 @@ xenFormatVif(virConf *conf,
             goto cleanup;
     }
 
-    if (netVal->list != NULL) {
-        int ret = virConfSetValue(conf, "vif", netVal);
-        netVal = NULL;
-        if (ret < 0)
-            goto cleanup;
-    }
+    if (netVal->list != NULL &&
+        virConfSetValue(conf, "vif", &netVal) < 0)
+        goto cleanup;
 
     return 0;
 

diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index 815c1216f7c9c860d6f8b0a133e0fa2714f23585..6e489f35ad54e9982ae553da6efaa0b91aadaccf 100644 (file)
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1447,12 +1447,9 @@ xenFormatXLDomainVnuma(virConf *conf,
             goto cleanup;
     }
 
-    if (vnumaVal->list != NULL) {
-        int ret = virConfSetValue(conf, "vnuma", vnumaVal);
-            vnumaVal = NULL;
-            if (ret < 0)
-                return -1;
-    }
+    if (vnumaVal->list != NULL &&
+        virConfSetValue(conf, "vnuma", &vnumaVal) < 0)
+        return -1;
 
     return 0;
 
@@ -1697,12 +1694,9 @@ xenFormatXLDomainDisks(virConf *conf, virDomainDef *def)
             goto cleanup;
     }
 
-    if (diskVal->list != NULL) {
-        int ret = virConfSetValue(conf, "disk", diskVal);
-        diskVal = NULL;
-        if (ret < 0)
-            return -1;
-    }
+    if (diskVal->list != NULL &&
+        virConfSetValue(conf, "disk", &diskVal) < 0)
+        return -1;
 
     return 0;
 
@@ -1848,11 +1842,8 @@ xenFormatXLInputDevs(virConf *conf, virDomainDef *def)
                 if (xenConfigSetString(conf, "usbdevice", usbdevices->list->str) < 0)
                     goto error;
             } else {
-                if (virConfSetValue(conf, "usbdevice", usbdevices) < 0) {
-                    usbdevices = NULL;
+                if (virConfSetValue(conf, "usbdevice", &usbdevices) < 0)
                     goto error;
-                }
-                usbdevices = NULL;
             }
         }
     }
@@ -1923,12 +1914,9 @@ xenFormatXLUSBController(virConf *conf,
         }
     }
 
-    if (usbctrlVal->list != NULL) {
-        int ret = virConfSetValue(conf, "usbctrl", usbctrlVal);
-        usbctrlVal = NULL;
-        if (ret < 0)
-            return -1;
-    }
+    if (usbctrlVal->list != NULL &&
+        virConfSetValue(conf, "usbctrl", &usbctrlVal) < 0)
+        return -1;
 
     return 0;
 
@@ -1985,12 +1973,9 @@ xenFormatXLUSB(virConf *conf,
         }
     }
 
-    if (usbVal->list != NULL) {
-        int ret = virConfSetValue(conf, "usbdev", usbVal);
-        usbVal = NULL;
-        if (ret < 0)
-            return -1;
-    }
+    if (usbVal->list != NULL &&
+        virConfSetValue(conf, "usbdev", &usbVal) < 0)
+        return -1;
 
     return 0;
 }
@@ -2057,12 +2042,9 @@ xenFormatXLDomainChannels(virConf *conf, virDomainDef *def)
             goto cleanup;
     }
 
-    if (channelVal->list != NULL) {
-        int ret = virConfSetValue(conf, "channel", channelVal);
-        channelVal = NULL;
-        if (ret < 0)
-            goto cleanup;
-    }
+    if (channelVal->list != NULL &&
+        virConfSetValue(conf, "channel", &channelVal) < 0)
+        goto cleanup;
 
     return 0;
 
@@ -2105,13 +2087,9 @@ xenFormatXLDomainNamespaceData(virConf *conf, virDomainDef *def)
             args->list = val;
     }
 
-    if (args->list != NULL) {
-        if (virConfSetValue(conf, "device_model_args", args) < 0) {
-            args = NULL;
-            goto error;
-        }
-        args = NULL;
-    }
+    if (args->list != NULL &&
+        virConfSetValue(conf, "device_model_args", &args) < 0)
+        goto error;
 
     return 0;
 

diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c
index b8f0471514c8379b57a246ec2bbf6276587228ec..f20307430c578478e1945e3c72331c6c4ccb0b0d 100644 (file)
--- a/src/libxl/xen_xm.c
+++ b/src/libxl/xen_xm.c
@@ -356,12 +356,9 @@ xenFormatXMDisks(virConf *conf, virDomainDef *def)
             goto cleanup;
     }
 
-    if (diskVal->list != NULL) {
-        int ret = virConfSetValue(conf, "disk", diskVal);
-        diskVal = NULL;
-        if (ret < 0)
-            goto cleanup;
-    }
+    if (diskVal->list != NULL &&
+        virConfSetValue(conf, "disk", &diskVal) < 0)
+        goto cleanup;
 
     return 0;
 

diff --git a/src/util/virconf.c b/src/util/virconf.c
index 29b36227911300aef7d11e72c0271fe247990b1d..cd45c5f657d857fcecdc33fa632e9f956459d3f4 100644 (file)
--- a/src/util/virconf.c
+++ b/src/util/virconf.c
@@ -1323,16 +1323,22 @@ int virConfGetValueULLong(virConf *conf,
 int
 virConfSetValue(virConf *conf,
                 const char *setting,
-                virConfValue *value)
+                virConfValue **value)
 {
     virConfEntry *cur;
     virConfEntry *prev = NULL;
 
-    if (value && value->type == VIR_CONF_STRING && value->str == NULL) {
+    if (!value) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("invalid use of conf API"));
+        return -1;
+    }
+
+    if (*value && (*value)->type == VIR_CONF_STRING && !(*value)->str) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("expecting a value for value of type %s"),
                        virConfTypeToString(VIR_CONF_STRING));
-        virConfFreeValue(value);
+        g_clear_pointer(value, virConfFreeValue);
         return -1;
     }
 
@@ -1348,7 +1354,7 @@ virConfSetValue(virConf *conf,
         cur = g_new0(virConfEntry, 1);
         cur->comment = NULL;
         cur->name = g_strdup(setting);
-        cur->value = value;
+        cur->value = g_steal_pointer(value);
         if (prev) {
             cur->next = prev->next;
             prev->next = cur;
@@ -1358,7 +1364,7 @@ virConfSetValue(virConf *conf,
         }
     } else {
         virConfFreeValue(cur->value);
-        cur->value = value;
+        cur->value = g_steal_pointer(value);
     }
     return 0;
 }

diff --git a/src/util/virconf.h b/src/util/virconf.h
index 558009b92e5dd228a3b1a5b4a573ba4df939b8f5..e656a6a815d61431043008d7faf97c5cf35f93c1 100644 (file)
--- a/src/util/virconf.h
+++ b/src/util/virconf.h
@@ -115,7 +115,7 @@ int virConfGetValueULLong(virConf *conf,
 
 int virConfSetValue(virConf *conf,
                     const char *setting,
-                    virConfValue *value);
+                    virConfValue **value);
 int virConfWalk(virConf *conf,
                 virConfWalkCallback callback,
                 void *opaque);