[PATCH 04/11] 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(a)redhat.com&gt; --- src/libxl/xen_common.c | 28 +++++++++++++--------------- src/libxl/xen_xl.c | 36 +++++++++++++----------------------- src/libxl/xen_xm.c | 4 +--- src/util/virconf.h | 1 + 4 files changed, 28 insertions(+), 41 deletions(-) diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index e68ca1cde3..f36e269aab 100644 --- 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 e3ddae8827..815c1216f7 100644 --- 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 9c2d091918..b8f0471514 100644 --- 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 937e5d43ed..558009b92e 100644 --- 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); -- 2.34.1