Patchwork qbus: fix memory leak in qbus_free()

login
register
mail settings
Submitter Isaku Yamahata
Date May 27, 2010, 5:35 a.m.
Message ID <20100527053558.GA31807@valinux.co.jp>
Download mbox | patch
Permalink /patch/53688/
State New
Headers show

Comments

Isaku Yamahata - May 27, 2010, 5:35 a.m.
BusState::name is allocated in qbus_create_inplace().
So it should be freed by qbus_free().

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/qdev.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Markus Armbruster - June 2, 2010, 3:01 p.m.
Isaku Yamahata <yamahata@valinux.co.jp> writes:

> BusState::name is allocated in qbus_create_inplace().
> So it should be freed by qbus_free().

Correct.

> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/qdev.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index af17486..2845af5 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -700,6 +700,7 @@ void qbus_free(BusState *bus)
>          QLIST_REMOVE(bus, sibling);
>          bus->parent->num_child_bus--;
>      }
> +    qemu_free((void*)bus->name);
>      if (bus->qdev_allocated) {
>          qemu_free(bus);
>      }

Ugly, superfluous cast to void *.

Thanks!
Isaku Yamahata - June 3, 2010, 12:59 a.m.
On Wed, Jun 02, 2010 at 05:01:13PM +0200, Markus Armbruster wrote:
> Isaku Yamahata <yamahata@valinux.co.jp> writes:
> 
> > BusState::name is allocated in qbus_create_inplace().
> > So it should be freed by qbus_free().
> 
> Correct.
> 
> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> > ---
> >  hw/qdev.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/qdev.c b/hw/qdev.c
> > index af17486..2845af5 100644
> > --- a/hw/qdev.c
> > +++ b/hw/qdev.c
> > @@ -700,6 +700,7 @@ void qbus_free(BusState *bus)
> >          QLIST_REMOVE(bus, sibling);
> >          bus->parent->num_child_bus--;
> >      }
> > +    qemu_free((void*)bus->name);
> >      if (bus->qdev_allocated) {
> >          qemu_free(bus);
> >      }
> 
> Ugly, superfluous cast to void *.

BusState::name is const char* so that the cast is necessary to drop const
qualifier. Otherwise gcc complains as follows.

qemu/hw/qdev.c: In function 'qbus_free':
qemu/hw/qdev.c:657: error: passing argument 1 of 'qemu_free' discards qualifiers from pointer target type
Markus Armbruster - June 3, 2010, 7:05 a.m.
Isaku Yamahata <yamahata@valinux.co.jp> writes:

> On Wed, Jun 02, 2010 at 05:01:13PM +0200, Markus Armbruster wrote:
>> Isaku Yamahata <yamahata@valinux.co.jp> writes:
>> 
>> > BusState::name is allocated in qbus_create_inplace().
>> > So it should be freed by qbus_free().
>> 
>> Correct.
>> 
>> > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
>> > ---
>> >  hw/qdev.c |    1 +
>> >  1 files changed, 1 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/hw/qdev.c b/hw/qdev.c
>> > index af17486..2845af5 100644
>> > --- a/hw/qdev.c
>> > +++ b/hw/qdev.c
>> > @@ -700,6 +700,7 @@ void qbus_free(BusState *bus)
>> >          QLIST_REMOVE(bus, sibling);
>> >          bus->parent->num_child_bus--;
>> >      }
>> > +    qemu_free((void*)bus->name);
>> >      if (bus->qdev_allocated) {
>> >          qemu_free(bus);
>> >      }
>> 
>> Ugly, superfluous cast to void *.
>
> BusState::name is const char* so that the cast is necessary to drop const
> qualifier. Otherwise gcc complains as follows.
>
> qemu/hw/qdev.c: In function 'qbus_free':
> qemu/hw/qdev.c:657: error: passing argument 1 of 'qemu_free' discards qualifiers from pointer target type

You're right.  I keep suppressing all the silly trouble caused by const.
Sorry for the noise.
Blue Swirl - June 10, 2010, 6:17 p.m.
Thanks, applied.

On Thu, May 27, 2010 at 5:35 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> BusState::name is allocated in qbus_create_inplace().
> So it should be freed by qbus_free().
>
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
> ---
>  hw/qdev.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/hw/qdev.c b/hw/qdev.c
> index af17486..2845af5 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -700,6 +700,7 @@ void qbus_free(BusState *bus)
>         QLIST_REMOVE(bus, sibling);
>         bus->parent->num_child_bus--;
>     }
> +    qemu_free((void*)bus->name);
>     if (bus->qdev_allocated) {
>         qemu_free(bus);
>     }
> --
> 1.6.6.1
>
>

Patch

diff --git a/hw/qdev.c b/hw/qdev.c
index af17486..2845af5 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -700,6 +700,7 @@  void qbus_free(BusState *bus)
         QLIST_REMOVE(bus, sibling);
         bus->parent->num_child_bus--;
     }
+    qemu_free((void*)bus->name);
     if (bus->qdev_allocated) {
         qemu_free(bus);
     }