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>
This commit is contained in:
Michal Privoznik 2022-01-14 10:11:42 +01:00
parent 94ce37c8e2
commit 17c324bd25
5 changed files with 46 additions and 78 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;
}

View File

@ -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);