8:39 a.m.
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(a)redhat.com>
---
src/libxl/xen_common.c | 37 +++++++++-----------------
src/libxl/xen_xl.c | 60 +++++++++++++-----------------------------
src/libxl/xen_xm.c | 9 +++----
src/util/virconf.c | 16 +++++++----
src/util/virconf.h | 2 +-
5 files changed, 46 insertions(+), 78 deletions(-)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index f36e269aab..2d363ac2fb 100644
--- 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 815c1216f7..6e489f35ad 100644
--- 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 b8f0471514..f20307430c 100644
--- 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 29b3622791..cd45c5f657 100644
--- 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 558009b92e..e656a6a815 100644
--- 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);
--
2.34.1