qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH] qdev: Fix memory leak


From: Stefan Weil
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] qdev: Fix memory leak
Date: Mon, 14 May 2012 21:36:36 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.16) Gecko/20120506 Iceowl/1.0b1 Icedove/3.0.11

Hello,

Am 03.05.2012 10:34, schrieb dunrong huang:
The str allocated in visit_type_str was not freed

Signed-off-by: dunrong huang <address@hidden>
---
hw/qdev-properties.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 98dd06a..8088699 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -726,7 +726,7 @@ static void set_mac(Object *obj, Visitor *v, void *opaque,
MACAddr *mac = qdev_get_prop_ptr(dev, prop);
Error *local_err = NULL;
int i, pos;
- char *str, *p;
+ char *str = NULL, *p;

Is this change really needed?


if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
@@ -753,10 +753,12 @@ static void set_mac(Object *obj, Visitor *v, void *opaque,
}
mac->a[i] = strtol(str+pos, &p, 16);
}
+ g_free(str);
return;

inval:
error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
+ g_free(str);
}

PropertyInfo qdev_prop_macaddr = {
@@ -825,7 +827,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
unsigned int slot, fn, n;
Error *local_err = NULL;
- char *str = (char *)"";
+ char *str = NULL;

As far as I could see, str does not need an initial value, so
neither the old nor the new code is optimal (both versions do
work).


if (dev->state != DEV_STATE_CREATED) {
error_set(errp, QERR_PERMISSION_DENIED);
@@ -847,10 +849,12 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
goto invalid;
}
*ptr = slot << 3 | fn;
+ g_free(str);
return;

invalid:
error_set_from_qdev_prop_error(errp, EINVAL, dev, prop, str);
+ g_free(str);
}

static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, size_t len)

Maybe some expert (Michael Roth?) can comment on the correct
usage of visit_type_str.

Is the initial value for the 2nd argument really needed?
The current QEMU code sometimes uses initialization,
but not always (see the code above, for example), so it is confusing.

Otherwise the patch is ok. It fixes memory leaks,
therefore it should be added to QEMU 1.1.

Tested-by: Stefan Weil <address@hidden>

Regards,

Stefan Weil




reply via email to

[Prev in Thread] Current Thread [Next in Thread]