src: Declare and use g_autoptr(virConfValue)

This commit declares g_autoptr() function for virConfValue type.
At the same time, it switches variable declarations to use it.
Also, in a few places we might have freed a variable twice, for
instance in xenFormatXLDomainNamespaceData(). This is because
virConfSetValue() consumes passed pointer (@value) even in case
of failure and thus any code that uses virConfSetValue() must
refrain from touching @value and it must not call
virConfFreeValue().

This semantic is not obvious and will be addressed in one of
future commits.

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 e68ca1cde32b16f2b173a39b14dd381fe605530a..f36e269aab6a5e222114d98477fd07eb958081fb 100644 (file)
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -1729,7 +1729,7 @@ xenFormatNet(virConnectPtr conn,
 static int
 xenFormatPCI(virConf *conf, virDomainDef *def)
 {
-    virConfValue *pciVal = NULL;
+    g_autoptr(virConfValue) pciVal = NULL;
     int hasPCI = 0;
     size_t i;
 
@@ -1794,7 +1794,6 @@ xenFormatPCI(virConf *conf, virDomainDef *def)
         if (ret < 0)
             return -1;
     }
-    VIR_FREE(pciVal);
 
     return 0;
 }
@@ -1970,7 +1969,7 @@ xenFormatCharDev(virConf *conf, virDomainDef *def,
             } else {
                 size_t j = 0;
                 int maxport = -1, port;
-                virConfValue *serialVal = NULL;
+                g_autoptr(virConfValue) serialVal = NULL;
 
                 if (STREQ(nativeFormat, XEN_CONFIG_FORMAT_XM)) {
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -1997,7 +1996,6 @@ xenFormatCharDev(virConf *conf, virDomainDef *def,
                     }
 
                     if (xenFormatSerial(serialVal, chr) < 0) {
-                        VIR_FREE(serialVal);
                         return -1;
                     }
                 }
@@ -2009,7 +2007,6 @@ xenFormatCharDev(virConf *conf, virDomainDef *def,
                     if (ret < 0)
                         return -1;
                 }
-                VIR_FREE(serialVal);
             }
         } else {
             if (xenConfigSetString(conf, "serial", "none") < 0)
@@ -2224,8 +2221,8 @@ xenFormatVfb(virConf *conf, virDomainDef *def)
                     return -1;
             }
         } else {
-            virConfValue *vfb;
-            virConfValue *disp;
+            g_autoptr(virConfValue) vfb = NULL;
+            g_autoptr(virConfValue) disp = NULL;
             char *vfbstr = NULL;
             g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 
@@ -2259,16 +2256,19 @@ xenFormatVfb(virConf *conf, virDomainDef *def)
 
             vfbstr = virBufferContentAndReset(&buf);
 
-            vfb = g_new0(virConfValue, 1);
             disp = g_new0(virConfValue, 1);
-
-            vfb->type = VIR_CONF_LIST;
-            vfb->list = disp;
             disp->type = VIR_CONF_STRING;
             disp->str = vfbstr;
 
-            if (virConfSetValue(conf, "vfb", vfb) < 0)
+            vfb = g_new0(virConfValue, 1);
+            vfb->type = VIR_CONF_LIST;
+            vfb->list = g_steal_pointer(&disp);
+
+            if (virConfSetValue(conf, "vfb", vfb) < 0) {
+                vfb = NULL;
                 return -1;
+            }
+            vfb = NULL;
         }
     }
 
@@ -2312,7 +2312,7 @@ xenFormatVif(virConf *conf,
              virDomainDef *def,
              const char *vif_typename)
 {
-    virConfValue *netVal = NULL;
+    g_autoptr(virConfValue) netVal = NULL;
     size_t i;
     int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
 
@@ -2333,11 +2333,9 @@ xenFormatVif(virConf *conf,
             goto cleanup;
     }
 
-    VIR_FREE(netVal);
     return 0;
 
  cleanup:
-    virConfFreeValue(netVal);
     return -1;
 }
 

diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index e3ddae88270e1bfe705464242a91981133ca87ef..815c1216f7c9c860d6f8b0a133e0fa2714f23585 100644 (file)
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -1429,7 +1429,7 @@ xenFormatXLDomainVnuma(virConf *conf,
                        virDomainDef *def)
 {
     virDomainNuma *numa = def->numa;
-    virConfValue *vnumaVal;
+    g_autoptr(virConfValue) vnumaVal = NULL;
     size_t i;
     size_t nr_nodes;
 
@@ -1453,12 +1453,10 @@ xenFormatXLDomainVnuma(virConf *conf,
             if (ret < 0)
                 return -1;
     }
-    VIR_FREE(vnumaVal);
 
     return 0;
 
  cleanup:
-    virConfFreeValue(vnumaVal);
     return -1;
 }
 
@@ -1683,7 +1681,7 @@ xenFormatXLDisk(virConfValue *list, virDomainDiskDef *disk)
 static int
 xenFormatXLDomainDisks(virConf *conf, virDomainDef *def)
 {
-    virConfValue *diskVal;
+    g_autoptr(virConfValue) diskVal = NULL;
     size_t i;
 
     diskVal = g_new0(virConfValue, 1);
@@ -1705,12 +1703,10 @@ xenFormatXLDomainDisks(virConf *conf, virDomainDef *def)
         if (ret < 0)
             return -1;
     }
-    VIR_FREE(diskVal);
 
     return 0;
 
  cleanup:
-    virConfFreeValue(diskVal);
     return -1;
 }
 
@@ -1807,7 +1803,7 @@ xenFormatXLInputDevs(virConf *conf, virDomainDef *def)
 {
     size_t i;
     const char *devtype;
-    virConfValue *usbdevices = NULL;
+    g_autoptr(virConfValue) usbdevices = NULL;
     virConfValue *lastdev;
 
     if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
@@ -1851,7 +1847,6 @@ xenFormatXLInputDevs(virConf *conf, virDomainDef *def)
                  * only one device present */
                 if (xenConfigSetString(conf, "usbdevice", usbdevices->list->str) < 0)
                     goto error;
-                virConfFreeValue(usbdevices);
             } else {
                 if (virConfSetValue(conf, "usbdevice", usbdevices) < 0) {
                     usbdevices = NULL;
@@ -1859,14 +1854,11 @@ xenFormatXLInputDevs(virConf *conf, virDomainDef *def)
                 }
                 usbdevices = NULL;
             }
-        } else {
-            VIR_FREE(usbdevices);
         }
     }
 
     return 0;
  error:
-    virConfFreeValue(usbdevices);
     return -1;
 }
 
@@ -1874,7 +1866,7 @@ static int
 xenFormatXLUSBController(virConf *conf,
                          virDomainDef *def)
 {
-    virConfValue *usbctrlVal = NULL;
+    g_autoptr(virConfValue) usbctrlVal = NULL;
     int hasUSBCtrl = 0;
     size_t i;
 
@@ -1937,12 +1929,10 @@ xenFormatXLUSBController(virConf *conf,
         if (ret < 0)
             return -1;
     }
-    VIR_FREE(usbctrlVal);
 
     return 0;
 
  error:
-    virConfFreeValue(usbctrlVal);
     return -1;
 }
 
@@ -1951,7 +1941,7 @@ static int
 xenFormatXLUSB(virConf *conf,
                virDomainDef *def)
 {
-    virConfValue *usbVal = NULL;
+    g_autoptr(virConfValue) usbVal = NULL;
     int hasUSB = 0;
     size_t i;
 
@@ -2001,7 +1991,6 @@ xenFormatXLUSB(virConf *conf,
         if (ret < 0)
             return -1;
     }
-    VIR_FREE(usbVal);
 
     return 0;
 }
@@ -2050,7 +2039,7 @@ xenFormatXLChannel(virConfValue *list, virDomainChrDef *channel)
 static int
 xenFormatXLDomainChannels(virConf *conf, virDomainDef *def)
 {
-    virConfValue *channelVal = NULL;
+    g_autoptr(virConfValue) channelVal = NULL;
     size_t i;
 
     channelVal = g_new0(virConfValue, 1);
@@ -2075,11 +2064,9 @@ xenFormatXLDomainChannels(virConf *conf, virDomainDef *def)
             goto cleanup;
     }
 
-    VIR_FREE(channelVal);
     return 0;
 
  cleanup:
-    virConfFreeValue(channelVal);
     return -1;
 }
 
@@ -2087,7 +2074,7 @@ static int
 xenFormatXLDomainNamespaceData(virConf *conf, virDomainDef *def)
 {
     libxlDomainXmlNsDef *nsdata = def->namespaceData;
-    virConfValue *args = NULL;
+    g_autoptr(virConfValue) args = NULL;
     size_t i;
 
     if (!nsdata)
@@ -2118,14 +2105,17 @@ xenFormatXLDomainNamespaceData(virConf *conf, virDomainDef *def)
             args->list = val;
     }
 
-    if (args->list != NULL)
-        if (virConfSetValue(conf, "device_model_args", args) < 0)
+    if (args->list != NULL) {
+        if (virConfSetValue(conf, "device_model_args", args) < 0) {
+            args = NULL;
             goto error;
+        }
+        args = NULL;
+    }
 
     return 0;
 
  error:
-    virConfFreeValue(args);
     return -1;
 }
 

diff --git a/src/libxl/xen_xm.c b/src/libxl/xen_xm.c
index 9c2d091918a8039abdc9cc500a5f0f306bfc85cc..b8f0471514c8379b57a246ec2bbf6276587228ec 100644 (file)
--- a/src/libxl/xen_xm.c
+++ b/src/libxl/xen_xm.c
@@ -340,7 +340,7 @@ xenFormatXMDisk(virConfValue *list,
 static int
 xenFormatXMDisks(virConf *conf, virDomainDef *def)
 {
-    virConfValue *diskVal = NULL;
+    g_autoptr(virConfValue) diskVal = NULL;
     size_t i = 0;
 
     diskVal = g_new0(virConfValue, 1);
@@ -362,12 +362,10 @@ xenFormatXMDisks(virConf *conf, virDomainDef *def)
         if (ret < 0)
             goto cleanup;
     }
-    VIR_FREE(diskVal);
 
     return 0;
 
  cleanup:
-    virConfFreeValue(diskVal);
     return -1;
 }
 

diff --git a/src/util/virconf.h b/src/util/virconf.h
index 937e5d43ed8aaa64d30b11156e17c341fffea781..558009b92e5dd228a3b1a5b4a573ba4df939b8f5 100644 (file)
--- a/src/util/virconf.h
+++ b/src/util/virconf.h
@@ -78,6 +78,7 @@ virConf *virConfReadString(const char *memory,
 int virConfFree(virConf *conf);
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virConf, virConfFree);
 void virConfFreeValue(virConfValue *val);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virConfValue, virConfFreeValue);
 virConfValue *virConfGetValue(virConf *conf,
                                 const char *setting);