NB, this series is targetting 10.0, NOT for 9.2 freeze.
With code like
Object *obj = object_new(TYPE_BLAH)
the caller can be pretty confident that they will successfully create
an object instance of TYPE_BLAH. They know exactly what type has been
requested, so it passing an abstract type for example, it is a clear
programmer error that they'll get an assertion failure.
Conversely with code like
void somefunc(const char *typename) {
Object * obj = object_new(typename)
...
}
all bets are off, because the call of object_new() knows nothing
about what 'typename' resolves to. It could easily be an abstract
type. As a result, many code paths have added a manual check ahead
of time
if (object_class_is_abstract(typename)) {
error_setg(errp, ....)
}
...except for where we forget to do this, such as qdev_new().
Overall 'object_new' is a bad design because it is inherantly
unsafe to call with unvalidated typenames.
This problem is made worse by the proposal to introduce the idea
of 'singleton' classes[1].
Thus, this series suggests a way to improve safety at build
time. The core idea is to allow 'object_new' to continue to be
used *if-and-only-if* given a static, const string, because that
scenario indicates the caller is aware of what type they are
creating at build time.
A new 'object_new_dynamic' method is proposed for cases where
the typename is dynamically chosen at runtime. This method has
an "Error **errp" parameter, which can report when an abstract
type is created, leaving the assert()s only for scenarios which
are unambiguous programmer errors.
With a little macro magic, we guarantee a compile error is
generated if 'object_new' is called with a dynamic type, forcing
all potentially unsafe code over to object_new_dynamic.
This is more tractable than adding 'Error **errp' to 'object_new'
as only a handful of places use a dynamic type name.
With this series, my objections to Peter Xu's singleton series[1]
would be largely nullified.
[1] https://lists.nongnu.org/archive/html/qemu-devel/2024-10/msg05524.html
Changed in v2:
* Removed "RFC" tag
* Converted code in all non-x86_64 targets
* Converted qdev_new to same pattern as object_new
* Ensured test suites work now
Daniel P. Berrangé (8):
qom: refactor checking abstract property when creating instances
qom: allow failure of object_new_with_class
qom: introduce object_new_dynamic()
convert code to object_new_dynamic() where appropriate
qom: enforce use of static, const string with object_new()
qom: introduce qdev_new_dynamic()
convert code to qdev_new_dynamic() where appropriate
hw: enforce use of static, const string with qdev_new()
accel/accel-user.c | 4 +-
chardev/char.c | 6 ++-
hw/arm/aspeed.c | 6 +--
hw/arm/exynos4210.c | 3 +-
hw/arm/highbank.c | 2 +-
hw/arm/integratorcp.c | 2 +-
hw/arm/mps3r.c | 3 +-
hw/arm/npcm7xx_boards.c | 2 +-
hw/arm/realview.c | 3 +-
hw/arm/sbsa-ref.c | 7 +--
hw/arm/versatilepb.c | 2 +-
hw/arm/vexpress.c | 4 +-
hw/arm/virt.c | 10 ++--
hw/arm/xilinx_zynq.c | 3 +-
hw/audio/soundhw.c | 2 +-
hw/block/xen-block.c | 7 ++-
hw/core/bus.c | 2 +-
hw/core/cpu-common.c | 2 +-
hw/core/qdev.c | 24 +++++++--
hw/core/sysbus.c | 2 +-
hw/i2c/core.c | 2 +-
hw/i386/x86-common.c | 5 +-
hw/i386/xen/xen-pvh.c | 2 +-
hw/intc/xics.c | 5 +-
hw/isa/isa-bus.c | 4 +-
hw/mips/cps.c | 3 +-
hw/pci-host/pnv_phb.c | 5 +-
hw/pci/pci.c | 2 +-
hw/ppc/e500.c | 2 +-
hw/ppc/pnv.c | 6 +--
hw/ppc/pnv_core.c | 5 +-
hw/ppc/spapr.c | 2 +-
hw/ppc/spapr_cpu_core.c | 5 +-
hw/ppc/spapr_drc.c | 2 +-
hw/s390x/s390-virtio-ccw.c | 10 +++-
hw/scsi/scsi-bus.c | 5 +-
hw/sparc/leon3.c | 2 +-
hw/sparc/sun4m.c | 2 +-
hw/sparc64/sparc64.c | 2 +-
hw/ssi/ssi.c | 2 +-
hw/vfio/common.c | 6 ++-
hw/vfio/container.c | 7 ++-
include/hw/qdev-core.h | 78 ++++++++++++++++++++++++++-
include/hw/usb.h | 4 +-
include/qom/object.h | 48 +++++++++++++++--
net/net.c | 7 +--
qom/object.c | 44 ++++++++++-----
qom/object_interfaces.c | 7 ++-
qom/qom-qmp-cmds.c | 16 +++---
system/qdev-monitor.c | 5 +-
system/vl.c | 6 ++-
target/arm/arm-qmp-cmds.c | 5 +-
target/i386/cpu-apic.c | 8 ++-
target/i386/cpu-sysemu.c | 11 ++--
target/i386/cpu.c | 4 +-
target/loongarch/loongarch-qmp-cmds.c | 5 +-
target/mips/cpu.c | 2 +-
target/riscv/riscv-qmp-cmds.c | 5 +-
target/s390x/cpu_models_sysemu.c | 7 ++-
target/xtensa/cpu.c | 2 +-
tests/unit/check-qom-interface.c | 3 +-
tests/unit/test-smp-parse.c | 20 +++----
62 files changed, 353 insertions(+), 116 deletions(-)
--
2.46.0
