8:20 a.m.
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
8:20 a.m.
New subject: [libvirt] [PATCH 1/2] uml: Avoid resource leak of event in umlInofityEvent
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
9:23 a.m.
New subject: [libvirt] [PATCH 1/2] uml: Avoid resource leak of event in umlInofityEvent
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
10:20 a.m.
New subject: [libvirt] [PATCH 1/2] uml: Avoid resource leak of event in umlInofityEvent
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
10:27 a.m.
New subject: [libvirt] [PATCH 1/2] uml: Avoid resource leak of event in umlInofityEvent
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
8:20 a.m.
New subject: [libvirt] [PATCH 2/2] parallels: Resolve some resource leaks
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
9:34 a.m.
New subject: [libvirt] [PATCH 2/2] parallels: Resolve some resource leaks
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
4322
days inactive
4322
days old
6 comments
3 participants
participants (3)
-
Eric Blake -
John Ferlan -
Peter Krempa