| Message ID | 20210924090427.9218-6-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qdev: Add JSON -device and fix QMP device_add | expand |
24.09.2021 12:04, Kevin Wolf wrote: > DeviceState.id is a pointer to a string that is stored in the QemuOpts > object DeviceState.opts and freed together with it. We want to create > devices without going through QemuOpts in the future, so make this a > separately allocated string. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Interesting that in hw/xen/xen-legacy-backend.c g_strdup_printf-allocated id is passed to qdev_set_id prior this patch. So, the patch seems to fix that small leak. Worth to mention? > --- > include/hw/qdev-core.h | 2 +- > include/monitor/qdev.h | 2 +- > hw/arm/virt.c | 2 +- > hw/core/qdev.c | 1 + > hw/pci-bridge/pci_expander_bridge.c | 2 +- > hw/ppc/e500.c | 2 +- > softmmu/qdev-monitor.c | 5 +++-- > 7 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > index 34c8a7506a..1857d9698e 100644 > --- a/include/hw/qdev-core.h > +++ b/include/hw/qdev-core.h > @@ -176,7 +176,7 @@ struct DeviceState { > Object parent_obj; > /*< public >*/ > > - const char *id; > + char *id; > char *canonical_path; > bool realized; > bool pending_deleted_event; > diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h > index eaa947d73a..389287eb44 100644 > --- a/include/monitor/qdev.h > +++ b/include/monitor/qdev.h > @@ -9,6 +9,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp); > > int qdev_device_help(QemuOpts *opts); > DeviceState *qdev_device_add(QemuOpts *opts, Error **errp); > -void qdev_set_id(DeviceState *dev, const char *id); > +void qdev_set_id(DeviceState *dev, char *id); > > #endif > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 1d59f0e59f..f933d48d3b 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1459,7 +1459,7 @@ static void create_platform_bus(VirtMachineState *vms) > MemoryRegion *sysmem = get_system_memory(); > > dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); > - dev->id = TYPE_PLATFORM_BUS_DEVICE; > + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); > qdev_prop_set_uint32(dev, "num_irqs", PLATFORM_BUS_NUM_IRQS); > qdev_prop_set_uint32(dev, "mmio_size", vms->memmap[VIRT_PLATFORM_BUS].size); > sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index cefc5eaa0a..d918b50a1d 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -956,6 +956,7 @@ static void device_finalize(Object *obj) > } > > qemu_opts_del(dev->opts); > + g_free(dev->id); > } > > static void device_class_base_init(ObjectClass *class, void *data) > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > index 7112dc3062..10e6e7c2ab 100644 > --- a/hw/pci-bridge/pci_expander_bridge.c > +++ b/hw/pci-bridge/pci_expander_bridge.c > @@ -245,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) > } else { > bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS); > bds = qdev_new("pci-bridge"); > - bds->id = dev_name; > + bds->id = g_strdup(dev_name); > qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr); > qdev_prop_set_bit(bds, PCI_BRIDGE_DEV_PROP_SHPC, false); > } > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > index 95451414dd..960e7efcd3 100644 > --- a/hw/ppc/e500.c > +++ b/hw/ppc/e500.c > @@ -1006,7 +1006,7 @@ void ppce500_init(MachineState *machine) > /* Platform Bus Device */ > if (pmc->has_platform_bus) { > dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); > - dev->id = TYPE_PLATFORM_BUS_DEVICE; > + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); > qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs); > qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size); > sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c > index 034b999401..1207e57a46 100644 > --- a/softmmu/qdev-monitor.c > +++ b/softmmu/qdev-monitor.c > @@ -592,7 +592,8 @@ static BusState *qbus_find(const char *path, Error **errp) > return bus; > } > > -void qdev_set_id(DeviceState *dev, const char *id) > +/* Takes ownership of @id, will be freed when deleting the device */ > +void qdev_set_id(DeviceState *dev, char *id) > { > if (id) { > dev->id = id; > @@ -690,7 +691,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > } > } > > - qdev_set_id(dev, qemu_opts_id(opts)); > + qdev_set_id(dev, g_strdup(qemu_opts_id(opts))); > > /* set properties */ > if (qemu_opt_foreach(opts, set_property, dev, errp)) { > In hw/pci/pci_bridge.c we do br->bus_name = dev->qdev.id It seems bad, as we now stole dynamic pointer, that may be freed later.
Am 24.09.2021 um 16:02 hat Vladimir Sementsov-Ogievskiy geschrieben: > 24.09.2021 12:04, Kevin Wolf wrote: > > DeviceState.id is a pointer to a string that is stored in the QemuOpts > > object DeviceState.opts and freed together with it. We want to create > > devices without going through QemuOpts in the future, so make this a > > separately allocated string. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > Interesting that in hw/xen/xen-legacy-backend.c > g_strdup_printf-allocated id is passed to qdev_set_id prior this > patch. So, the patch seems to fix that small leak. Worth to mention? Ok, I can mention it explicitly. > > --- > > include/hw/qdev-core.h | 2 +- > > include/monitor/qdev.h | 2 +- > > hw/arm/virt.c | 2 +- > > hw/core/qdev.c | 1 + > > hw/pci-bridge/pci_expander_bridge.c | 2 +- > > hw/ppc/e500.c | 2 +- > > softmmu/qdev-monitor.c | 5 +++-- > > 7 files changed, 9 insertions(+), 7 deletions(-) > > > > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h > > index 34c8a7506a..1857d9698e 100644 > > --- a/include/hw/qdev-core.h > > +++ b/include/hw/qdev-core.h > > @@ -176,7 +176,7 @@ struct DeviceState { > > Object parent_obj; > > /*< public >*/ > > - const char *id; > > + char *id; > > char *canonical_path; > > bool realized; > > bool pending_deleted_event; > > diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h > > index eaa947d73a..389287eb44 100644 > > --- a/include/monitor/qdev.h > > +++ b/include/monitor/qdev.h > > @@ -9,6 +9,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp); > > int qdev_device_help(QemuOpts *opts); > > DeviceState *qdev_device_add(QemuOpts *opts, Error **errp); > > -void qdev_set_id(DeviceState *dev, const char *id); > > +void qdev_set_id(DeviceState *dev, char *id); > > #endif > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index 1d59f0e59f..f933d48d3b 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -1459,7 +1459,7 @@ static void create_platform_bus(VirtMachineState *vms) > > MemoryRegion *sysmem = get_system_memory(); > > dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); > > - dev->id = TYPE_PLATFORM_BUS_DEVICE; > > + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); > > qdev_prop_set_uint32(dev, "num_irqs", PLATFORM_BUS_NUM_IRQS); > > qdev_prop_set_uint32(dev, "mmio_size", vms->memmap[VIRT_PLATFORM_BUS].size); > > sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > > index cefc5eaa0a..d918b50a1d 100644 > > --- a/hw/core/qdev.c > > +++ b/hw/core/qdev.c > > @@ -956,6 +956,7 @@ static void device_finalize(Object *obj) > > } > > qemu_opts_del(dev->opts); > > + g_free(dev->id); > > } > > static void device_class_base_init(ObjectClass *class, void *data) > > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c > > index 7112dc3062..10e6e7c2ab 100644 > > --- a/hw/pci-bridge/pci_expander_bridge.c > > +++ b/hw/pci-bridge/pci_expander_bridge.c > > @@ -245,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) > > } else { > > bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS); > > bds = qdev_new("pci-bridge"); > > - bds->id = dev_name; > > + bds->id = g_strdup(dev_name); > > qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr); > > qdev_prop_set_bit(bds, PCI_BRIDGE_DEV_PROP_SHPC, false); > > } > > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c > > index 95451414dd..960e7efcd3 100644 > > --- a/hw/ppc/e500.c > > +++ b/hw/ppc/e500.c > > @@ -1006,7 +1006,7 @@ void ppce500_init(MachineState *machine) > > /* Platform Bus Device */ > > if (pmc->has_platform_bus) { > > dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); > > - dev->id = TYPE_PLATFORM_BUS_DEVICE; > > + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); > > qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs); > > qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size); > > sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); > > diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c > > index 034b999401..1207e57a46 100644 > > --- a/softmmu/qdev-monitor.c > > +++ b/softmmu/qdev-monitor.c > > @@ -592,7 +592,8 @@ static BusState *qbus_find(const char *path, Error **errp) > > return bus; > > } > > -void qdev_set_id(DeviceState *dev, const char *id) > > +/* Takes ownership of @id, will be freed when deleting the device */ > > +void qdev_set_id(DeviceState *dev, char *id) > > { > > if (id) { > > dev->id = id; > > @@ -690,7 +691,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) > > } > > } > > - qdev_set_id(dev, qemu_opts_id(opts)); > > + qdev_set_id(dev, g_strdup(qemu_opts_id(opts))); > > /* set properties */ > > if (qemu_opt_foreach(opts, set_property, dev, errp)) { > > > > In hw/pci/pci_bridge.c > > we do > > br->bus_name = dev->qdev.id > > It seems bad, as we now stole dynamic pointer, that may be freed later. If it's bad now, it was as bad before because previously it was just a pointer stolen from the QemuOpts that is freed in the same function as dev->id after this patch. I think the code is correct (it's just another field of the same device, the real bus name is just a copy of it, and the bus can't outlive the device that owns it anyway), but it might be bad style. Kevin
24.09.2021 18:10, Kevin Wolf wrote: > Am 24.09.2021 um 16:02 hat Vladimir Sementsov-Ogievskiy geschrieben: >> 24.09.2021 12:04, Kevin Wolf wrote: >>> DeviceState.id is a pointer to a string that is stored in the QemuOpts >>> object DeviceState.opts and freed together with it. We want to create >>> devices without going through QemuOpts in the future, so make this a >>> separately allocated string. >>> >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >> >> Interesting that in hw/xen/xen-legacy-backend.c >> g_strdup_printf-allocated id is passed to qdev_set_id prior this >> patch. So, the patch seems to fix that small leak. Worth to mention? > > Ok, I can mention it explicitly. > >>> --- >>> include/hw/qdev-core.h | 2 +- >>> include/monitor/qdev.h | 2 +- >>> hw/arm/virt.c | 2 +- >>> hw/core/qdev.c | 1 + >>> hw/pci-bridge/pci_expander_bridge.c | 2 +- >>> hw/ppc/e500.c | 2 +- >>> softmmu/qdev-monitor.c | 5 +++-- >>> 7 files changed, 9 insertions(+), 7 deletions(-) >>> >>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h >>> index 34c8a7506a..1857d9698e 100644 >>> --- a/include/hw/qdev-core.h >>> +++ b/include/hw/qdev-core.h >>> @@ -176,7 +176,7 @@ struct DeviceState { >>> Object parent_obj; >>> /*< public >*/ >>> - const char *id; >>> + char *id; >>> char *canonical_path; >>> bool realized; >>> bool pending_deleted_event; >>> diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h >>> index eaa947d73a..389287eb44 100644 >>> --- a/include/monitor/qdev.h >>> +++ b/include/monitor/qdev.h >>> @@ -9,6 +9,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp); >>> int qdev_device_help(QemuOpts *opts); >>> DeviceState *qdev_device_add(QemuOpts *opts, Error **errp); >>> -void qdev_set_id(DeviceState *dev, const char *id); >>> +void qdev_set_id(DeviceState *dev, char *id); >>> #endif >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 1d59f0e59f..f933d48d3b 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -1459,7 +1459,7 @@ static void create_platform_bus(VirtMachineState *vms) >>> MemoryRegion *sysmem = get_system_memory(); >>> dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); >>> - dev->id = TYPE_PLATFORM_BUS_DEVICE; >>> + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); >>> qdev_prop_set_uint32(dev, "num_irqs", PLATFORM_BUS_NUM_IRQS); >>> qdev_prop_set_uint32(dev, "mmio_size", vms->memmap[VIRT_PLATFORM_BUS].size); >>> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c >>> index cefc5eaa0a..d918b50a1d 100644 >>> --- a/hw/core/qdev.c >>> +++ b/hw/core/qdev.c >>> @@ -956,6 +956,7 @@ static void device_finalize(Object *obj) >>> } >>> qemu_opts_del(dev->opts); >>> + g_free(dev->id); >>> } >>> static void device_class_base_init(ObjectClass *class, void *data) >>> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c >>> index 7112dc3062..10e6e7c2ab 100644 >>> --- a/hw/pci-bridge/pci_expander_bridge.c >>> +++ b/hw/pci-bridge/pci_expander_bridge.c >>> @@ -245,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) >>> } else { >>> bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS); >>> bds = qdev_new("pci-bridge"); >>> - bds->id = dev_name; >>> + bds->id = g_strdup(dev_name); >>> qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr); >>> qdev_prop_set_bit(bds, PCI_BRIDGE_DEV_PROP_SHPC, false); >>> } >>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c >>> index 95451414dd..960e7efcd3 100644 >>> --- a/hw/ppc/e500.c >>> +++ b/hw/ppc/e500.c >>> @@ -1006,7 +1006,7 @@ void ppce500_init(MachineState *machine) >>> /* Platform Bus Device */ >>> if (pmc->has_platform_bus) { >>> dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); >>> - dev->id = TYPE_PLATFORM_BUS_DEVICE; >>> + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); >>> qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs); >>> qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size); >>> sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); >>> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c >>> index 034b999401..1207e57a46 100644 >>> --- a/softmmu/qdev-monitor.c >>> +++ b/softmmu/qdev-monitor.c >>> @@ -592,7 +592,8 @@ static BusState *qbus_find(const char *path, Error **errp) >>> return bus; >>> } >>> -void qdev_set_id(DeviceState *dev, const char *id) >>> +/* Takes ownership of @id, will be freed when deleting the device */ >>> +void qdev_set_id(DeviceState *dev, char *id) >>> { >>> if (id) { >>> dev->id = id; >>> @@ -690,7 +691,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) >>> } >>> } >>> - qdev_set_id(dev, qemu_opts_id(opts)); >>> + qdev_set_id(dev, g_strdup(qemu_opts_id(opts))); >>> /* set properties */ >>> if (qemu_opt_foreach(opts, set_property, dev, errp)) { >>> >> >> In hw/pci/pci_bridge.c >> >> we do >> >> br->bus_name = dev->qdev.id >> >> It seems bad, as we now stole dynamic pointer, that may be freed later. > > If it's bad now, it was as bad before because previously it was just a > pointer stolen from the QemuOpts that is freed in the same function as > dev->id after this patch. > Ah, right, OK. > I think the code is correct (it's just another field of the same device, > the real bus name is just a copy of it, and the bus can't outlive the > device that owns it anyway), but it might be bad style. > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 34c8a7506a..1857d9698e 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -176,7 +176,7 @@ struct DeviceState { Object parent_obj; /*< public >*/ - const char *id; + char *id; char *canonical_path; bool realized; bool pending_deleted_event; diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h index eaa947d73a..389287eb44 100644 --- a/include/monitor/qdev.h +++ b/include/monitor/qdev.h @@ -9,6 +9,6 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp); int qdev_device_help(QemuOpts *opts); DeviceState *qdev_device_add(QemuOpts *opts, Error **errp); -void qdev_set_id(DeviceState *dev, const char *id); +void qdev_set_id(DeviceState *dev, char *id); #endif diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 1d59f0e59f..f933d48d3b 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1459,7 +1459,7 @@ static void create_platform_bus(VirtMachineState *vms) MemoryRegion *sysmem = get_system_memory(); dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); - dev->id = TYPE_PLATFORM_BUS_DEVICE; + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); qdev_prop_set_uint32(dev, "num_irqs", PLATFORM_BUS_NUM_IRQS); qdev_prop_set_uint32(dev, "mmio_size", vms->memmap[VIRT_PLATFORM_BUS].size); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); diff --git a/hw/core/qdev.c b/hw/core/qdev.c index cefc5eaa0a..d918b50a1d 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -956,6 +956,7 @@ static void device_finalize(Object *obj) } qemu_opts_del(dev->opts); + g_free(dev->id); } static void device_class_base_init(ObjectClass *class, void *data) diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c index 7112dc3062..10e6e7c2ab 100644 --- a/hw/pci-bridge/pci_expander_bridge.c +++ b/hw/pci-bridge/pci_expander_bridge.c @@ -245,7 +245,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp) } else { bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS); bds = qdev_new("pci-bridge"); - bds->id = dev_name; + bds->id = g_strdup(dev_name); qdev_prop_set_uint8(bds, PCI_BRIDGE_DEV_PROP_CHASSIS_NR, pxb->bus_nr); qdev_prop_set_bit(bds, PCI_BRIDGE_DEV_PROP_SHPC, false); } diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 95451414dd..960e7efcd3 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -1006,7 +1006,7 @@ void ppce500_init(MachineState *machine) /* Platform Bus Device */ if (pmc->has_platform_bus) { dev = qdev_new(TYPE_PLATFORM_BUS_DEVICE); - dev->id = TYPE_PLATFORM_BUS_DEVICE; + dev->id = g_strdup(TYPE_PLATFORM_BUS_DEVICE); qdev_prop_set_uint32(dev, "num_irqs", pmc->platform_bus_num_irqs); qdev_prop_set_uint32(dev, "mmio_size", pmc->platform_bus_size); sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal); diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c index 034b999401..1207e57a46 100644 --- a/softmmu/qdev-monitor.c +++ b/softmmu/qdev-monitor.c @@ -592,7 +592,8 @@ static BusState *qbus_find(const char *path, Error **errp) return bus; } -void qdev_set_id(DeviceState *dev, const char *id) +/* Takes ownership of @id, will be freed when deleting the device */ +void qdev_set_id(DeviceState *dev, char *id) { if (id) { dev->id = id; @@ -690,7 +691,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp) } } - qdev_set_id(dev, qemu_opts_id(opts)); + qdev_set_id(dev, g_strdup(qemu_opts_id(opts))); /* set properties */ if (qemu_opt_foreach(opts, set_property, dev, errp)) {
DeviceState.id is a pointer to a string that is stored in the QemuOpts object DeviceState.opts and freed together with it. We want to create devices without going through QemuOpts in the future, so make this a separately allocated string. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/hw/qdev-core.h | 2 +- include/monitor/qdev.h | 2 +- hw/arm/virt.c | 2 +- hw/core/qdev.c | 1 + hw/pci-bridge/pci_expander_bridge.c | 2 +- hw/ppc/e500.c | 2 +- softmmu/qdev-monitor.c | 5 +++-- 7 files changed, 9 insertions(+), 7 deletions(-)