| Message ID | 20220402154711.679252-1-laurent@vivier.eu |
|---|---|
| State | New |
| Headers | show |
| Series | nubus: fix memory leak in nubus_bridge_init() | expand |
On 02/04/2022 16:47, Laurent Vivier wrote: > Move qbus_init() to a realize function. > > Leak detected by valgrind: > > QTEST_QEMU_BINARY="valgrind --leak-check=full \ > --show-leak-kinds=definite ./qemu-system-m68k" \ > tests/qtest/device-introspect-test \ > -p /m68k/device/introspect/concrete/defaults/none > ... > ==606164== 6 bytes in 1 blocks are definitely lost in loss record 123 of 1,771 > ==606164== at 0x484486F: malloc (vg_replace_malloc.c:381) > ==606164== by 0x4FF0428: g_malloc (gmem.c:106) > ==606164== by 0x5004433: g_strdup (gstrfuncs.c:364) > ==606164== by 0x3D1BA6: memory_region_do_init (memory.c:1170) > ==606164== by 0x3D1BA6: memory_region_init (memory.c:1195) > ==606164== by 0x375C88: nubus_init (nubus-bus.c:104) > ==606164== by 0x49DE82: object_init_with_type (object.c:377) > ==606164== by 0x49DE82: object_initialize_with_type (object.c:519) > ==606164== by 0x495772: qbus_init (bus.c:158) > ==606164== by 0x375DDB: nubus_bridge_init (nubus-bridge.c:21) > ==606164== by 0x49DE82: object_init_with_type (object.c:377) > ==606164== by 0x49DE82: object_initialize_with_type (object.c:519) > ==606164== by 0x49E028: object_new_with_type (object.c:734) > ==606164== by 0x5660AC: qmp_device_list_properties (qom-qmp-cmds.c:146) > ==606164== by 0x67CC35: qmp_marshal_device_list_properties (qapi-commands-qdev.c:66) > ... > > Reported-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- > hw/nubus/nubus-bridge.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/hw/nubus/nubus-bridge.c b/hw/nubus/nubus-bridge.c > index a42c86080f25..51c3f6328784 100644 > --- a/hw/nubus/nubus-bridge.c > +++ b/hw/nubus/nubus-bridge.c > @@ -18,11 +18,17 @@ static void nubus_bridge_init(Object *obj) > NubusBridge *s = NUBUS_BRIDGE(obj); > NubusBus *bus = &s->bus; > > - qbus_init(bus, sizeof(s->bus), TYPE_NUBUS_BUS, DEVICE(s), NULL); > - > qdev_init_gpio_out(DEVICE(s), bus->irqs, NUBUS_IRQS); > } > > +static void nubus_bridge_realize(DeviceState *d, Error **errp) > +{ > + NubusBridge *s = NUBUS_BRIDGE(d); > + NubusBus *bus = &s->bus; > + > + qbus_init(bus, sizeof(NubusBus), TYPE_NUBUS_BUS, d, NULL); > +} > + > static Property nubus_bridge_properties[] = { > DEFINE_PROP_UINT16("slot-available-mask", NubusBridge, > bus.slot_available_mask, 0xffff), > @@ -34,6 +40,7 @@ static void nubus_bridge_class_init(ObjectClass *klass, void *data) > DeviceClass *dc = DEVICE_CLASS(klass); > > dc->fw_name = "nubus"; > + dc->realize = nubus_bridge_realize; > device_class_set_props(dc, nubus_bridge_properties); > } Since qbus_init() is adding a bus that can have child devices, I think the correct solution here would be to add an instance_finalize function to the Nubus bridge device that calls qbus_finalize(). ATB, Mark.
diff --git a/hw/nubus/nubus-bridge.c b/hw/nubus/nubus-bridge.c index a42c86080f25..51c3f6328784 100644 --- a/hw/nubus/nubus-bridge.c +++ b/hw/nubus/nubus-bridge.c @@ -18,11 +18,17 @@ static void nubus_bridge_init(Object *obj) NubusBridge *s = NUBUS_BRIDGE(obj); NubusBus *bus = &s->bus; - qbus_init(bus, sizeof(s->bus), TYPE_NUBUS_BUS, DEVICE(s), NULL); - qdev_init_gpio_out(DEVICE(s), bus->irqs, NUBUS_IRQS); } +static void nubus_bridge_realize(DeviceState *d, Error **errp) +{ + NubusBridge *s = NUBUS_BRIDGE(d); + NubusBus *bus = &s->bus; + + qbus_init(bus, sizeof(NubusBus), TYPE_NUBUS_BUS, d, NULL); +} + static Property nubus_bridge_properties[] = { DEFINE_PROP_UINT16("slot-available-mask", NubusBridge, bus.slot_available_mask, 0xffff), @@ -34,6 +40,7 @@ static void nubus_bridge_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc->fw_name = "nubus"; + dc->realize = nubus_bridge_realize; device_class_set_props(dc, nubus_bridge_properties); }
Move qbus_init() to a realize function. Leak detected by valgrind: QTEST_QEMU_BINARY="valgrind --leak-check=full \ --show-leak-kinds=definite ./qemu-system-m68k" \ tests/qtest/device-introspect-test \ -p /m68k/device/introspect/concrete/defaults/none ... ==606164== 6 bytes in 1 blocks are definitely lost in loss record 123 of 1,771 ==606164== at 0x484486F: malloc (vg_replace_malloc.c:381) ==606164== by 0x4FF0428: g_malloc (gmem.c:106) ==606164== by 0x5004433: g_strdup (gstrfuncs.c:364) ==606164== by 0x3D1BA6: memory_region_do_init (memory.c:1170) ==606164== by 0x3D1BA6: memory_region_init (memory.c:1195) ==606164== by 0x375C88: nubus_init (nubus-bus.c:104) ==606164== by 0x49DE82: object_init_with_type (object.c:377) ==606164== by 0x49DE82: object_initialize_with_type (object.c:519) ==606164== by 0x495772: qbus_init (bus.c:158) ==606164== by 0x375DDB: nubus_bridge_init (nubus-bridge.c:21) ==606164== by 0x49DE82: object_init_with_type (object.c:377) ==606164== by 0x49DE82: object_initialize_with_type (object.c:519) ==606164== by 0x49E028: object_new_with_type (object.c:734) ==606164== by 0x5660AC: qmp_device_list_properties (qom-qmp-cmds.c:146) ==606164== by 0x67CC35: qmp_marshal_device_list_properties (qapi-commands-qdev.c:66) ... Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu> --- hw/nubus/nubus-bridge.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)