> -----Original Message----- > From: Peter Maydell [mailto:peter.mayd...@linaro.org] > Sent: Tuesday, December 15, 2020 12:20 AM > To: ganqixin <ganqi...@huawei.com> > Cc: qemu-arm <qemu-...@nongnu.org>; QEMU Developers > <qemu-devel@nongnu.org>; Chenqun (kuhn) > <kuhn.chen...@huawei.com>; Zhanghailiang > <zhang.zhanghaili...@huawei.com>; Euler Robot > <euler.ro...@huawei.com>; Beniamino Galvani <b.galv...@gmail.com> > Subject: Re: [PATCH 1/7] allwinner-a10-pit: Use ptimer_free() in the > finalize function to avoid memleaks > > On Mon, 14 Dec 2020 at 16:02, Peter Maydell <peter.mayd...@linaro.org> > wrote: > > > > On Fri, 27 Nov 2020 at 07:19, Gan Qixin <ganqi...@huawei.com> wrote: > > > > > > When running device-introspect-test, a memory leak occurred in the > > > a10_pit_init function, so use ptimer_free() in the finalize function to > avoid it. > > > > > > ASAN shows memory leak stack: > > > > > > Indirect leak of 288 byte(s) in 6 object(s) allocated from: > > > #0 0xffffab97e1f0 in __interceptor_calloc > (/lib64/libasan.so.5+0xee1f0) > > > #1 0xffffab256800 in g_malloc0 (/lib64/libglib-2.0.so.0+0x56800) > > > #2 0xaaabf555db84 in timer_new_full > /qemu/include/qemu/timer.h:523 > > > #3 0xaaabf555db84 in timer_new > /qemu/include/qemu/timer.h:544 > > > #4 0xaaabf555db84 in timer_new_ns > /qemu/include/qemu/timer.h:562 > > > #5 0xaaabf555db84 in ptimer_init /qemu/hw/core/ptimer.c:433 > > > #6 0xaaabf57415e8 in a10_pit_init > /qemu/hw/timer/allwinner-a10-pit.c:278 > > > #7 0xaaabf6339f6c in object_initialize_with_type > /qemu/qom/object.c:515 > > > #8 0xaaabf633ca04 in object_initialize_child_with_propsv > /qemu/qom/object.c:564 > > > #9 0xaaabf633cc08 in object_initialize_child_with_props > /qemu/qom/object.c:547 > > > #10 0xaaabf5b94680 in aw_a10_init > /qemu/hw/arm/allwinner-a10.c:49 > > > #11 0xaaabf6339f6c in object_initialize_with_type > /qemu/qom/object.c:515 > > > #12 0xaaabf633a1e0 in object_new_with_type > > > /qemu/qom/object.c:729 > > > > > > Reported-by: Euler Robot <euler.ro...@huawei.com> > > > Signed-off-by: Gan Qixin <ganqi...@huawei.com> > > > --- > > > Cc: Beniamino Galvani <b.galv...@gmail.com> > > > --- > > > hw/timer/allwinner-a10-pit.c | 21 ++++++++++++++++----- > > > 1 file changed, 16 insertions(+), 5 deletions(-) > > > > > > diff --git a/hw/timer/allwinner-a10-pit.c > > > b/hw/timer/allwinner-a10-pit.c index f84fc0ea25..be211983b0 > 100644 > > > --- a/hw/timer/allwinner-a10-pit.c > > > +++ b/hw/timer/allwinner-a10-pit.c > > > @@ -279,6 +279,16 @@ static void a10_pit_init(Object *obj) > > > } > > > } > > > > > > +static void a10_pit_finalize(Object *obj) { > > > + AwA10PITState *s = AW_A10_PIT(obj); > > > + int i; > > > + > > > + for (i = 0; i < AW_A10_PIT_TIMER_NR; i++) { > > > + ptimer_free(s->timer[i]); > > > + } > > > +} > > > + > > > static void a10_pit_class_init(ObjectClass *klass, void *data) { > > > DeviceClass *dc = DEVICE_CLASS(klass); @@ -290,11 +300,12 > @@ > > > static void a10_pit_class_init(ObjectClass *klass, void *data) } > > > > > > static const TypeInfo a10_pit_info = { > > > - .name = TYPE_AW_A10_PIT, > > > - .parent = TYPE_SYS_BUS_DEVICE, > > > - .instance_size = sizeof(AwA10PITState), > > > - .instance_init = a10_pit_init, > > > - .class_init = a10_pit_class_init, > > > + .name = TYPE_AW_A10_PIT, > > > + .parent = TYPE_SYS_BUS_DEVICE, > > > + .instance_size = sizeof(AwA10PITState), > > > + .instance_init = a10_pit_init, > > > + .instance_finalize = a10_pit_finalize, > > > + .class_init = a10_pit_class_init, > > > }; > > > > Please don't make unrelated whitespace changes like this in a patch. > > We don't line up the assignments in this sort of struct -- this is > > deliberate, so that if a new line is added whose field name happens to > > be longer than those used already, the patch does not have to touch > > all the lines in the struct to maintain the formatting. > > Instead you get a readable diff where only the new line changes, not > > all the others. > > Hmm. Having said that I see that the other 6 devices touched by this series > did use the line-up-the-assignments style. > Anyway, the style this device was using is the right one.
Thanks for your reply, I will resend this patch. By the way, do we need to keep the other 6 devices in the same style as this device? Gan Qixin
