[libvirt] [PATCH 0/2] Resending Resolve RESOURCE_LEAK issues found by Coverity
This were initially part of the following series: https://www.redhat.com/archives/libvir-list/2013-January/msg00529.html Specifically https://www.redhat.com/archives/libvir-list/2013-January/msg00537.html https://www.redhat.com/archives/libvir-list/2013-January/msg00541.html Rather than bury them under a reply - I'll just pop them out again John Ferlan (2): uml: Avoid resource leak of event in umlInofityEvent parallels: Resolve some resource leaks src/parallels/parallels_driver.c | 30 +++++++++++++++++++----------- src/uml/uml_driver.c | 6 ++++-- 2 files changed, 23 insertions(+), 13 deletions(-) -- 1.7.11.7
If there was more than one inotify_event found in the read/while loop, then only the last event found would have been queued. --- src/uml/uml_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index c6fef69..705495e 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -410,11 +410,13 @@ reread: } if (dom) virObjectUnlock(dom); + if (event) { + umlDomainEventQueue(driver, event); + event = NULL; + } } cleanup: - if (event) - umlDomainEventQueue(driver, event); umlDriverUnlock(driver); } -- 1.7.11.7
On 01/22/13 15:20, John Ferlan wrote:
If there was more than one inotify_event found in the read/while loop, then only the last event found would have been queued. --- src/uml/uml_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index c6fef69..705495e 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -410,11 +410,13 @@ reread: } if (dom) virObjectUnlock(dom); + if (event) { + umlDomainEventQueue(driver, event); + event = NULL; + }
Yeah, it's in a big loop. Took me a while to notice.
}
cleanup: - if (event) - umlDomainEventQueue(driver, event); umlDriverUnlock(driver); }
ACK. Peter
On 01/22/2013 08:23 AM, Peter Krempa wrote:
On 01/22/13 15:20, John Ferlan wrote:
If there was more than one inotify_event found in the read/while loop, then only the last event found would have been queued. --- src/uml/uml_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
ACK.
Pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/22/13 17:20, Eric Blake wrote:
On 01/22/2013 08:23 AM, Peter Krempa wrote:
On 01/22/13 15:20, John Ferlan wrote:
If there was more than one inotify_event found in the read/while loop, then only the last event found would have been queued. --- src/uml/uml_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
ACK.
Pushed.
Uhm, I was just going to do that, but the build test was finishing. Peter
Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within no_memory label to be consistent Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is allocated locally. --- src/parallels/parallels_driver.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index ea193af..8beab2c 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -304,8 +304,9 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value) no_memory: virReportOOMError(); -cleanup: + VIR_FREE(accel); virDomainVideoDefFree(video); +cleanup: return -1; } @@ -1809,58 +1810,58 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, if (!create && oldnet->type != newnet->type) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network type is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->model, newnet->model)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network device model is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->data.network.portgroup, newnet->data.network.portgroup)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network portgroup is not supported")); - return -1; + goto error; } if (!virNetDevVPortProfileEqual(oldnet->virtPortProfile, newnet->virtPortProfile)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing virtual port profile is not supported")); - return -1; + goto error; } if (newnet->tune.sndbuf_specified) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting send buffer size is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->script, newnet->script)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting startup script is not supported")); - return -1; + goto error; } if (!STREQ_NULLABLE(oldnet->filter, newnet->filter)) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing filter params is not supported")); - return -1; + goto error; } if (newnet->bandwidth != NULL) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting bandwidth params is not supported")); - return -1; + goto error; } for (i = 0; i < sizeof(newnet->vlan); i++) { if (((char *)&newnet->vlan)[i] != 0) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Setting vlan params is not supported")); - return -1; + goto error; } } @@ -1902,15 +1903,22 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, is_changed = true; } + if (create) + VIR_FREE(oldnet); + if (!create && !is_changed) { /* nothing changed - no need to run prlctl */ return 0; } if (virCommandRun(cmd, NULL)) - return -1; + goto error; return 0; +error: + if (create) + VIR_FREE(oldnet); + return -1; } static int -- 1.7.11.7
On 01/22/13 15:20, John Ferlan wrote:
Be sure to VIR_FREE(accel) and moved virDomainVideoDefFree() within no_memory label to be consistent
Resolve resource leak in parallelsApplyIfaceParams() when the 'oldnet' is allocated locally. --- src/parallels/parallels_driver.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index ea193af..8beab2c 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -304,8 +304,9 @@ parallelsAddVideoInfo(virDomainDefPtr def, virJSONValuePtr value)
no_memory: virReportOOMError(); -cleanup: + VIR_FREE(accel); virDomainVideoDefFree(video); +cleanup:
Hm, this label is used only in error cases. "error:" would be probably better here. But it is pre-existing.
return -1; }
@@ -1809,58 +1810,58 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, if (!create && oldnet->type != newnet->type) { virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("Changing network type is not supported"));
[...]
@@ -1902,15 +1903,22 @@ static int parallelsApplyIfaceParams(parallelsDomObjPtr pdom, is_changed = true; }
+ if (create) + VIR_FREE(oldnet); + if (!create && !is_changed) { /* nothing changed - no need to run prlctl */ return 0; }
if (virCommandRun(cmd, NULL)) - return -1; + goto error;
return 0;
Shouldn't cmd be freed here too?
+error: + if (create) + VIR_FREE(oldnet); + return -1; }
static int
Peter
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa