2:45 p.m.
Ryota Ozaki wrote:
diff --git a/qemud/qemud.c b/qemud/qemud.c
index df275e6..17ba44a 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -1733,7 +1733,7 @@ readmore:
/* Possibly need to create another receive buffer */
if ((client->nrequests < max_client_requests &&
- VIR_ALLOC(client->rx) < 0)) {
+ !client->rx && VIR_ALLOC(client->rx) < 0)) {
qemudDispatchClientFailure(client);
} else {
if (client->rx)
Hm, I'm not super familiar with this section of code, but this doesn't seem to
be right. How can client->rx ever be NULL here? We dereference
client->rx->bufferOffset very early on in the function, so we would have crashed
before we got here. What am I missing?
diff --git a/src/domain_conf.c b/src/domain_conf.c
index 1d2cc7c..4b64219 100644
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -4361,6 +4361,7 @@ int virDomainSaveXML(virConnectPtr conn,
cleanup:
if (fd != -1)
close(fd);
+ VIR_FREE(configFile);
return ret;
}
This one looks good.
diff --git a/src/network_conf.c b/src/network_conf.c
index bb649a4..58a4f32 100644
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -820,6 +820,7 @@ int virNetworkDeleteConfig(virConnectPtr conn,
{
char *configFile = NULL;
char *autostartLink = NULL;
+ int ret = -1;
if ((configFile = virNetworkConfigFile(conn, configDir,
net->def->name)) == NULL)
goto error;
@@ -836,12 +837,12 @@ int virNetworkDeleteConfig(virConnectPtr conn,
goto error;
}
- return 0;
+ ret = 0;
error:
VIR_FREE(configFile);
VIR_FREE(autostartLink);
- return -1;
+ return ret;
}
This one looks good.
char *virNetworkConfigFile(virConnectPtr conn,
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index 22f5edd..32d6a48 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -1034,7 +1034,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
virDomainNetDefPtr net,
int qemuCmdFlags)
{
- char *brname;
+ char *brname = NULL;
int err;
int tapfd = -1;
int vnet_hdr = 0;
@@ -1053,7 +1053,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
if (brname == NULL)
return -1;
} else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
- brname = net->data.bridge.brname;
+ brname = strdup(net->data.bridge.brname);
} else {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Network type %d is not supported"),
net->type);
@@ -1063,7 +1063,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
if (!driver->brctl && (err = brInit(&driver->brctl))) {
virReportSystemError(conn, err, "%s",
_("cannot initialize bridge support"));
- return -1;
+ goto cleanup;
}
if (!net->ifname ||
@@ -1072,7 +1072,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
VIR_FREE(net->ifname);
if (!(net->ifname = strdup("vnet%d"))) {
virReportOOMError(conn);
- return -1;
+ goto cleanup;
}
/* avoid exposing vnet%d in dumpxml or error outputs */
template_ifname = 1;
@@ -1100,9 +1100,12 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
}
if (template_ifname)
VIR_FREE(net->ifname);
- return -1;
+ tapfd = -1;
}
+cleanup:
+ VIR_FREE(brname);
+
return tapfd;
}
This generally looks OK.
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index ca6d329..222e591 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -856,6 +856,8 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
vol->type = VIR_STORAGE_VOL_FILE;
vol->target.format = VIR_STORAGE_VOL_FILE_RAW; /* Real value
is filled in during probe */
+ if (vol->target.path)
+ VIR_FREE(vol->target.path);
Again, how can this ever be the case? The first time through the loop vol is
definitely NULL, so there's no possibility vol->target.path contains any
allocated memory. At the end of this loop, we assign the whole vol object to a
slot in the pool->volumes array, set vol to NULL, and then subsequent iterations
get a new vol object allocated. So how can this ever be true?
if (virAsprintf(&vol->target.path,
"%s/%s",
pool->def->target.path,
vol->name) == -1)
@@ -1022,6 +1024,9 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
vol->type = VIR_STORAGE_VOL_FILE;
+ if (vol->target.path)
+ VIR_FREE(vol->target.path);
+
if (virAsprintf(&vol->target.path, "%s/%s",
pool->def->target.path,
vol->name) == -1) {
I'm not sure either way on this one, I didn't follow the code path back far
enough. What situations can lead to this?
--
Chris Lalancette