| Message ID | 6f0b480e1f0fbeb9550d447dcbeda920f1869d2d.1726598846.git.quic_mathbern@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw: fix memory leak in IRQState allocation | expand |
On Tue, 17 Sep 2024, Matheus Tavares Bernardino wrote: > At e72a7f65c1 (hw: Move declaration of IRQState to header and add init > function, 2024-06-29), we've changed qemu_allocate_irq() to use a > combination of g_new() + object_initialize() instead of > IRQ(object_new()). The latter sets obj->free, so that that the memory is > properly cleaned when the object is finalized, but the former doesn't. > > Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> Sorry for breaking it, I did not know about that feature of object_new. Thanks for fixing it. Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> Fixes: e72a7f65c1 (hw: Move declaration of IRQState to header and add init function) Regards, BALATON Zoltan > --- > hw/core/irq.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/hw/core/irq.c b/hw/core/irq.c > index db95ffc18f..7d80de1ca6 100644 > --- a/hw/core/irq.c > +++ b/hw/core/irq.c > @@ -34,13 +34,19 @@ void qemu_set_irq(qemu_irq irq, int level) > irq->handler(irq->opaque, irq->n, level); > } > > +static void qemu_init_irq_fields(IRQState *irq, qemu_irq_handler handler, > + void *opaque, int n) > +{ > + irq->handler = handler; > + irq->opaque = opaque; > + irq->n = n; > +} > + > void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque, > int n) > { > object_initialize(irq, sizeof(*irq), TYPE_IRQ); > - irq->handler = handler; > - irq->opaque = opaque; > - irq->n = n; > + qemu_init_irq_fields(irq, handler, opaque, n); > } > > qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, > @@ -66,11 +72,8 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) > > qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n) > { > - IRQState *irq; > - > - irq = g_new(IRQState, 1); > - qemu_init_irq(irq, handler, opaque, n); > - > + IRQState *irq = IRQ(object_new(TYPE_IRQ)); > + qemu_init_irq_fields(irq, handler, opaque, n); > return irq; > } > >
On 9/17/2024 1:47 PM, Matheus Tavares Bernardino wrote: > At e72a7f65c1 (hw: Move declaration of IRQState to header and add init > function, 2024-06-29), we've changed qemu_allocate_irq() to use a > combination of g_new() + object_initialize() instead of > IRQ(object_new()). The latter sets obj->free, so that that the memory is > properly cleaned when the object is finalized, but the former doesn't. > > Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> > --- > hw/core/irq.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/hw/core/irq.c b/hw/core/irq.c > index db95ffc18f..7d80de1ca6 100644 > --- a/hw/core/irq.c > +++ b/hw/core/irq.c > @@ -34,13 +34,19 @@ void qemu_set_irq(qemu_irq irq, int level) > irq->handler(irq->opaque, irq->n, level); > } > > +static void qemu_init_irq_fields(IRQState *irq, qemu_irq_handler handler, > + void *opaque, int n) This function was named as if it should be public, but declared `static`. I think it might make more sense to call this function "init_irq_fields" instead. > +{ > + irq->handler = handler; > + irq->opaque = opaque; > + irq->n = n; > +} > + > void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque, > int n) > { > object_initialize(irq, sizeof(*irq), TYPE_IRQ); > - irq->handler = handler; > - irq->opaque = opaque; > - irq->n = n; > + qemu_init_irq_fields(irq, handler, opaque, n); > } > > qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, > @@ -66,11 +72,8 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) > > qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n) > { > - IRQState *irq; > - > - irq = g_new(IRQState, 1); > - qemu_init_irq(irq, handler, opaque, n); > - > + IRQState *irq = IRQ(object_new(TYPE_IRQ)); > + qemu_init_irq_fields(irq, handler, opaque, n); > return irq; > } >
diff --git a/hw/core/irq.c b/hw/core/irq.c index db95ffc18f..7d80de1ca6 100644 --- a/hw/core/irq.c +++ b/hw/core/irq.c @@ -34,13 +34,19 @@ void qemu_set_irq(qemu_irq irq, int level) irq->handler(irq->opaque, irq->n, level); } +static void qemu_init_irq_fields(IRQState *irq, qemu_irq_handler handler, + void *opaque, int n) +{ + irq->handler = handler; + irq->opaque = opaque; + irq->n = n; +} + void qemu_init_irq(IRQState *irq, qemu_irq_handler handler, void *opaque, int n) { object_initialize(irq, sizeof(*irq), TYPE_IRQ); - irq->handler = handler; - irq->opaque = opaque; - irq->n = n; + qemu_init_irq_fields(irq, handler, opaque, n); } qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler, @@ -66,11 +72,8 @@ qemu_irq *qemu_allocate_irqs(qemu_irq_handler handler, void *opaque, int n) qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n) { - IRQState *irq; - - irq = g_new(IRQState, 1); - qemu_init_irq(irq, handler, opaque, n); - + IRQState *irq = IRQ(object_new(TYPE_IRQ)); + qemu_init_irq_fields(irq, handler, opaque, n); return irq; }
At e72a7f65c1 (hw: Move declaration of IRQState to header and add init function, 2024-06-29), we've changed qemu_allocate_irq() to use a combination of g_new() + object_initialize() instead of IRQ(object_new()). The latter sets obj->free, so that that the memory is properly cleaned when the object is finalized, but the former doesn't. Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> --- hw/core/irq.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)