mbox series

[00/16] qom/object: Deprecate type_register()

Message ID 20241029085934.2799066-1-zhao1.liu@intel.com (mailing list archive)
Headers show
Series qom/object: Deprecate type_register() | expand

Message

Zhao Liu Oct. 29, 2024, 8:59 a.m. UTC
Hi maintainers,

This series is trying to deprecate type_register() and just keep
type_register_static() to register QOM type.

This series chosen to deprecate type_register() since changes required
to deprecate type_register() are smaller. (type_register_static() needs
1000+ LOC changes.)

The two main changes are patch 15 and 16, while the others are trivial
replacements.

This series is based on commit fdf250e5a37830 ("Merge tag
'pull-maintainer-oct-misc-241024-1' of https://gitlab.com/stsquad/qemu
into staging").


Introduction
============

The type_register() and type_register_static() have existed since the
birth of QOM (commit 2f28d2ff9dce ("qom: add the base Object class
(v2)")).

In the code implementation, type_register_static() has always been just
a wrapper around type_register(), and they are essentially the same.

The only difference between them is described in the documentation 
(include/qom/object.h):

* type_register_static()

> @info and all of the strings it points to should exist for the life time
> that the type is registered.

* type_register()

> Unlike type_register_static(), this call does not require @info or its
> string members to continue to exist after the call returns.

From the documentation, the difference between these two interfaces
arises from the lifetime of 2 cases:

1. the strings contained in the TypeInfo parameter.

The *_static variant requires the strings to have a long lifetime
(static).

I reviewed the discussions on the mailing list about the QOM v1 patch
[1], and I understand that the consideration for static is due to
Anthony's idea that in certain cases, the string field could be "const
char *", allowing the address to be directly copied to the created
Type/TypeImpl.

However, this consideration seems unnecessary in the merged v2 version,
as Anthony followed Paolo's suggestion to pass all string fields by
copying them via g_strdup() to the created TypeImple. This remains true
to this day.

[1]: https://lore.kernel.org/qemu-devel/4EF1EEA4.40209@us.ibm.com/


2. the function pointer and a special item called "class_data" in
TypeInfo

I suppose that there are currently no lifetime issues about these items
in QEMU, as neither type_register() nor type_register_static()
explicitly checks whether the parameters are static. If there were any
issues, they would likely be easily detected.

Furthermore, I haven't seen any preference for these items in the usage
of type_register() and type_register_static().


Based on points 1 and 2, I think it is sufficient to explain that
type_register() and type_register_static() are redundant in usage and do
not require distinction. Additionally, since they are consistent in the
code, it is safe to deprecate either one.

Since the changes required to deprecate type_register() are smaller,
choose to deprecate type_register() and delete the requirement about
string lifetime from the documentation.


Thanks and Best Regards,
Zhao
---
Zhao Liu (16):
  arm: Replace type_register() with type_register_static()
  hw/block: Replace type_register() with type_register_static()
  hw/net: Replace type_register() with type_register_static()
  ppc: Replace type_register() with type_register_static()
  hw/rtc: Replace type_register() with type_register_static()
  hw/scsi: Replace type_register() with type_register_static()
  hw/sensor: Replace type_register() with type_register_static()
  hw/usb: Replace type_register() with type_register_static()
  hw/virtio: Replace type_register() with type_register_static()
  i386: Replace type_register() with type_register_static()
  target/mips: Replace type_register() with type_register_static()
  target/sparc: Replace type_register() with type_register_static()
  target/xtensa: Replace type_register() with type_register_static()
  ui: Replace type_register() with type_register_static()
  script/codeconverter/qom_type_info: Deprecate MakeTypeRegisterStatic
    and MakeTypeRegisterNotStatic
  qom/object: Deprecate type_register()

 hw/arm/armsse.c                               |  2 +-
 hw/arm/smmuv3.c                               |  4 ++--
 hw/block/m25p80.c                             |  2 +-
 hw/net/e1000.c                                |  2 +-
 hw/net/eepro100.c                             |  2 +-
 hw/ppc/spapr.c                                |  2 +-
 hw/rtc/m48t59-isa.c                           |  2 +-
 hw/rtc/m48t59.c                               |  2 +-
 hw/scsi/megasas.c                             |  2 +-
 hw/scsi/mptsas.c                              |  2 +-
 hw/sensor/tmp421.c                            |  2 +-
 hw/usb/hcd-ehci-pci.c                         |  2 +-
 hw/usb/hcd-uhci.c                             |  2 +-
 hw/virtio/virtio-pci.c                        |  8 ++++----
 include/hw/i386/pc.h                          |  4 ++--
 include/qom/object.h                          | 14 -------------
 qom/object.c                                  |  7 +------
 .../codeconverter/qom_type_info.py            | 20 -------------------
 target/arm/cpu.c                              |  2 +-
 target/arm/cpu64.c                            |  2 +-
 target/i386/cpu.c                             |  2 +-
 target/mips/cpu.c                             |  2 +-
 target/ppc/kvm.c                              |  2 +-
 target/sparc/cpu.c                            |  2 +-
 target/xtensa/helper.c                        |  2 +-
 ui/console-vc.c                               |  2 +-
 ui/dbus.c                                     |  2 +-
 ui/gtk.c                                      |  2 +-
 ui/spice-app.c                                |  2 +-
 29 files changed, 32 insertions(+), 71 deletions(-)