On Mon, May 14, 2012 at 09:36:36PM +0200, Stefan Weil wrote:
> 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.