>-----Original Message----- >From: Philippe Mathieu-Daudé [mailto:phi...@redhat.com] >Sent: Wednesday, February 12, 2020 2:16 PM >To: Chenqun (kuhn) <kuhn.chen...@huawei.com>; qemu- >de...@nongnu.org; i.mitsya...@gmail.com; peter.mayd...@linaro.org >Cc: qemu-triv...@nongnu.org; Zhanghailiang ><zhang.zhanghaili...@huawei.com> >Subject: Re: [PATCH] hw/char/exynos4210_uart: Fix memleaks in >exynos4210_uart_init > >On 2/12/20 4:36 AM, kuhn.chen...@huawei.com wrote: >> From: Chen Qun <kuhn.chen...@huawei.com> >> >> It's easy to reproduce as follow: >> virsh qemu-monitor-command vm1 --pretty '{"execute": >> "device-list-properties", "arguments":{"typename":"exynos4210.uart"}}' >> >> ASAN shows memory leak stack: >> #1 0xfffd896d71cb in g_malloc0 (/lib64/libglib-2.0.so.0+0x571cb) >> #2 0xaaad270beee3 in timer_new_full /qemu/include/qemu/timer.h:530 >> #3 0xaaad270beee3 in timer_new /qemu/include/qemu/timer.h:551 >> #4 0xaaad270beee3 in timer_new_ns /qemu/include/qemu/timer.h:569 >> #5 0xaaad270beee3 in exynos4210_uart_init >/qemu/hw/char/exynos4210_uart.c:677 >> #6 0xaaad275c8f4f in object_initialize_with_type /qemu/qom/object.c:516 >> #7 0xaaad275c91bb in object_new_with_type /qemu/qom/object.c:684 >> #8 0xaaad2755df2f in qmp_device_list_properties >> /qemu/qom/qom-qmp-cmds.c:152 >> >> Reported-by: Euler Robot <euler.ro...@huawei.com> >> Signed-off-by: Chen Qun <kuhn.chen...@huawei.com> >> --- >> hw/char/exynos4210_uart.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/hw/char/exynos4210_uart.c b/hw/char/exynos4210_uart.c >> index 25d6588e41..5048db5410 100644 >> --- a/hw/char/exynos4210_uart.c >> +++ b/hw/char/exynos4210_uart.c >> @@ -674,10 +674,6 @@ static void exynos4210_uart_init(Object *obj) >> SysBusDevice *dev = SYS_BUS_DEVICE(obj); >> Exynos4210UartState *s = EXYNOS4210_UART(dev); >> >> - s->fifo_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, >> - exynos4210_uart_timeout_int, s); >> - s->wordtime = NANOSECONDS_PER_SECOND * 10 / 9600; > >Why are you moving s->wordtime from init() to realize()? > Hi Philippe, thanks for your reply!
Because I found the variable wordtime is usually used with fifo_timeout_timer. Eg, they are used together in the exynos4210_uart_rx_timeout_set function. I didn't find anything wrong with wordtime in the realize(). Does it have any other effects? Thanks. >> - >> /* memory mapping */ >> memory_region_init_io(&s->iomem, obj, &exynos4210_uart_ops, s, >> "exynos4210.uart", >> EXYNOS4210_UART_REGS_MEM_SIZE); @@ -691,6 +687,10 @@ static void >exynos4210_uart_realize(DeviceState *dev, Error **errp) >> { >> Exynos4210UartState *s = EXYNOS4210_UART(dev); >> >> + s->fifo_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, >> + exynos4210_uart_timeout_int, s); >> + s->wordtime = NANOSECONDS_PER_SECOND * 10 / 9600; >> + >> qemu_chr_fe_set_handlers(&s->chr, exynos4210_uart_can_receive, >> exynos4210_uart_receive, >> exynos4210_uart_event, >> NULL, s, NULL, true); >>