[RFC PATCH-for-5.2 0/5] qom: Let ObjectPropertyGet functions return a boolean value

Philippe Mathieu-Daudé posted 5 patches 5 years, 4 months ago
Test checkpatch passed
Test docker-mingw@fedora failed
Test FreeBSD passed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200715175835.27744-1-philmd@redhat.com
include/hw/qdev-core.h           |   4 +-
include/qom/object.h             |  50 +++++++++-----
accel/kvm/kvm-all.c              |   4 +-
accel/tcg/tcg-all.c              |   4 +-
authz/list.c                     |   4 +-
backends/cryptodev.c             |   4 +-
backends/hostmem-file.c          |   4 +-
backends/hostmem-memfd.c         |   4 +-
backends/hostmem.c               |  13 ++--
backends/tpm/tpm_util.c          |   7 +-
block/throttle-groups.c          |   8 +--
bootdevice.c                     |   4 +-
chardev/char-socket.c            |   4 +-
crypto/secret_keyring.c          |   5 +-
hw/acpi/ich9.c                   |   4 +-
hw/arm/virt.c                    |   4 +-
hw/block/xen-block.c             |   9 ++-
hw/core/machine.c                |   4 +-
hw/core/qdev-properties-system.c |  33 +++++----
hw/core/qdev-properties.c        |  89 ++++++++++++------------
hw/core/qdev.c                   |   5 +-
hw/cpu/core.c                    |   8 +--
hw/gpio/aspeed_gpio.c            |   8 +--
hw/i386/microvm.c                |  12 ++--
hw/i386/pc.c                     |  12 ++--
hw/i386/x86.c                    |   8 +--
hw/ide/qdev.c                    |   4 +-
hw/intc/apic_common.c            |   4 +-
hw/mem/nvdimm.c                  |  11 ++-
hw/mem/pc-dimm.c                 |   6 +-
hw/misc/aspeed_sdmc.c            |   4 +-
hw/misc/pca9552.c                |   8 +--
hw/misc/tmp105.c                 |   4 +-
hw/misc/tmp421.c                 |   8 +--
hw/net/ne2000-isa.c              |   4 +-
hw/pci-host/i440fx.c             |  16 ++---
hw/pci-host/q35.c                |  16 ++---
hw/ppc/spapr_caps.c              |  21 +++---
hw/ppc/spapr_drc.c               |  21 +++---
hw/riscv/sifive_u.c              |   4 +-
hw/s390x/css.c                   |   4 +-
hw/s390x/s390-pci-bus.c          |   4 +-
hw/usb/dev-storage.c             |   4 +-
hw/vfio/pci-quirks.c             |  14 ++--
hw/virtio/virtio-balloon.c       |  16 +++--
hw/virtio/virtio-mem.c           |  17 ++---
iothread.c                       |   4 +-
net/colo-compare.c               |   8 +--
net/dump.c                       |   4 +-
net/filter-buffer.c              |   4 +-
qom/object.c                     | 115 ++++++++++++++++---------------
softmmu/memory.c                 |  14 ++--
target/arm/cpu64.c               |  14 ++--
target/i386/cpu.c                |  48 ++++++++-----
target/ppc/compat.c              |   4 +-
target/s390x/cpu.c               |  11 +--
target/s390x/cpu_models.c        |  14 ++--
target/sparc/cpu.c               |   4 +-
58 files changed, 397 insertions(+), 352 deletions(-)
[RFC PATCH-for-5.2 0/5] qom: Let ObjectPropertyGet functions return a boolean value
Posted by Philippe Mathieu-Daudé 5 years, 4 months ago
RFC series to follow Markus direction to simplify error
propagation. Not sure it is worth it yet. It starts to
be interesting when using the QEMU_WARN_UNUSED_RESULT
attribute in the visitors, such:

-- >8 --
@@ -525,6 +533,7 @@ bool visit_type_uint8(Visitor *v, const char *name, uint8_t *obj,
  * Visit a uint16_t value.
  * Like visit_type_int(), except clamps the value to uint16_t range.
  */
+QEMU_WARN_UNUSED_RESULT
 bool visit_type_uint16(Visitor *v, const char *name, uint16_t *obj,
                        Error **errp);

---

But to get there we need to update the QAPI generators first :)

Philippe Mathieu-Daudé (5):
  hw/core/qdev-properties: Simplify get_reserved_region()
  qom: Split ObjectPropertyAccessor as ObjectProperty[Get/Set]
  qom: Use g_autofree in ObjectPropertyGet functions
  qom: Let ObjectPropertyGet functions return a boolean value
  hw/virtio: Simplify virtio_mem_set_requested_size()

 include/hw/qdev-core.h           |   4 +-
 include/qom/object.h             |  50 +++++++++-----
 accel/kvm/kvm-all.c              |   4 +-
 accel/tcg/tcg-all.c              |   4 +-
 authz/list.c                     |   4 +-
 backends/cryptodev.c             |   4 +-
 backends/hostmem-file.c          |   4 +-
 backends/hostmem-memfd.c         |   4 +-
 backends/hostmem.c               |  13 ++--
 backends/tpm/tpm_util.c          |   7 +-
 block/throttle-groups.c          |   8 +--
 bootdevice.c                     |   4 +-
 chardev/char-socket.c            |   4 +-
 crypto/secret_keyring.c          |   5 +-
 hw/acpi/ich9.c                   |   4 +-
 hw/arm/virt.c                    |   4 +-
 hw/block/xen-block.c             |   9 ++-
 hw/core/machine.c                |   4 +-
 hw/core/qdev-properties-system.c |  33 +++++----
 hw/core/qdev-properties.c        |  89 ++++++++++++------------
 hw/core/qdev.c                   |   5 +-
 hw/cpu/core.c                    |   8 +--
 hw/gpio/aspeed_gpio.c            |   8 +--
 hw/i386/microvm.c                |  12 ++--
 hw/i386/pc.c                     |  12 ++--
 hw/i386/x86.c                    |   8 +--
 hw/ide/qdev.c                    |   4 +-
 hw/intc/apic_common.c            |   4 +-
 hw/mem/nvdimm.c                  |  11 ++-
 hw/mem/pc-dimm.c                 |   6 +-
 hw/misc/aspeed_sdmc.c            |   4 +-
 hw/misc/pca9552.c                |   8 +--
 hw/misc/tmp105.c                 |   4 +-
 hw/misc/tmp421.c                 |   8 +--
 hw/net/ne2000-isa.c              |   4 +-
 hw/pci-host/i440fx.c             |  16 ++---
 hw/pci-host/q35.c                |  16 ++---
 hw/ppc/spapr_caps.c              |  21 +++---
 hw/ppc/spapr_drc.c               |  21 +++---
 hw/riscv/sifive_u.c              |   4 +-
 hw/s390x/css.c                   |   4 +-
 hw/s390x/s390-pci-bus.c          |   4 +-
 hw/usb/dev-storage.c             |   4 +-
 hw/vfio/pci-quirks.c             |  14 ++--
 hw/virtio/virtio-balloon.c       |  16 +++--
 hw/virtio/virtio-mem.c           |  17 ++---
 iothread.c                       |   4 +-
 net/colo-compare.c               |   8 +--
 net/dump.c                       |   4 +-
 net/filter-buffer.c              |   4 +-
 qom/object.c                     | 115 ++++++++++++++++---------------
 softmmu/memory.c                 |  14 ++--
 target/arm/cpu64.c               |  14 ++--
 target/i386/cpu.c                |  48 ++++++++-----
 target/ppc/compat.c              |   4 +-
 target/s390x/cpu.c               |  11 +--
 target/s390x/cpu_models.c        |  14 ++--
 target/sparc/cpu.c               |   4 +-
 58 files changed, 397 insertions(+), 352 deletions(-)

-- 
2.21.3


Re: [RFC PATCH-for-5.2 0/5] qom: Let ObjectPropertyGet functions return a boolean value
Posted by Markus Armbruster 5 years, 4 months ago
Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> RFC series to follow Markus direction to simplify error
> propagation. Not sure it is worth it yet. It starts to
> be interesting when using the QEMU_WARN_UNUSED_RESULT
> attribute in the visitors, such:
>
> -- >8 --
> @@ -525,6 +533,7 @@ bool visit_type_uint8(Visitor *v, const char *name, uint8_t *obj,
>   * Visit a uint16_t value.
>   * Like visit_type_int(), except clamps the value to uint16_t range.
>   */
> +QEMU_WARN_UNUSED_RESULT
>  bool visit_type_uint16(Visitor *v, const char *name, uint16_t *obj,
>                         Error **errp);

QEMU_WARN_UNUSED_RESULT is problematic with functions taking an Error **
argument, because using &error_abort or &error_fatal the intended way
triggers the warning.

Does that make &error_abort and &error_fatal bad ideas?  They do help
keeping the code concise in places...  Hundreds of places, according to
git-grep.

> ---
>
> But to get there we need to update the QAPI generators first :)