This is a different take on an RFE I posted in July: https://www.redhat.com/archives/libvir-list/2018-July/msg01815.html The goal of this series is to get us close to being able to raise errors from ToString and FromString functions directly. This will allow us to drop many hundreds of lines of error reporting and accompanying translator load. The first attempt above added a new macro, VIR_ENUM_IMPL_LABEL, which would take a string label, like 'domain type' for virDomainType enum. The generated ToString and FromString functions for that enum would now raise errors containing the label. Redundant error reporting could now be stripped out piece by piece as individual enums are converted. Thinking some more though I don't really like this strategy. Calling code won't have an easy way to know whether ToString/FromString functions are the type that raise errors or not, which could lead to new code accidentally omitting them and failing to raise an error. It could also stretch out the time that the code base is in a half transitioned state. This series is a slightly different direction. VIR_ENUM_IMPL now always takes a label, but for now we don't actually raise any errors. Later at a chosen time we can enable error reporting in the code and begin removing duplicate error reporting. The only downside is that until the transition is complete, there may be cases of double error reporting, but that's the lesser of two evils compared to the previous approach IMO. So the feedback I'm looking for: - Is this worth doing? (danpb+mprivozn said as much for first posting) - Is this approach acceptable? - Review of the strings added in the last patch Notes: - This is on top of pkrempa's virenum.c patches on list - The last two patches are separated to make review easier, but they will be committed together - When enabled, one notable case this will make error reporting worse is virTristate usage. But I think if we manually implement *TOString/FromString wrappers in that case we can require callers to pass in an identifying string. Something to think about for later Cole Robinson (4): Always put _LAST enums on second line of VIR_ENUM_IMPL cfg.mk: Only force _LAST enum on VIR_ENUM_IMPL second line util: Add 'label' field to VIR_ENUM_IMPL Add string labels to all VIR_ENUM_IMPL calls cfg.mk | 6 +- docs/apibuild.py | 12 ++ src/access/viraccessperm.c | 20 +- src/conf/capabilities.c | 3 +- src/conf/cpu_conf.c | 21 +- src/conf/device_conf.c | 3 +- src/conf/domain_capabilities.c | 3 +- src/conf/domain_conf.c | 333 +++++++++++++++++++----------- src/conf/interface_conf.c | 2 +- src/conf/netdev_vlan_conf.c | 3 +- src/conf/network_conf.c | 11 +- src/conf/node_device_conf.c | 12 +- src/conf/numa_conf.c | 7 +- src/conf/nwfilter_conf.c | 21 +- src/conf/snapshot_conf.c | 6 +- src/conf/storage_adapter_conf.c | 2 +- src/conf/storage_conf.c | 18 +- src/libxl/libxl_domain.c | 3 +- src/locking/lock_daemon.c | 3 +- src/logging/log_daemon.c | 3 +- src/lxc/lxc_domain.c | 7 +- src/lxc/lxc_native.c | 3 +- src/network/leaseshelper.c | 3 +- src/qemu/qemu_agent.c | 4 +- src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_command.c | 27 ++- src/qemu/qemu_domain.c | 12 +- src/qemu/qemu_driver.c | 6 +- src/qemu/qemu_firmware.c | 6 +- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_migration_cookie.c | 2 +- src/qemu/qemu_migration_params.c | 9 +- src/qemu/qemu_monitor.c | 8 +- src/qemu/qemu_monitor_json.c | 10 +- src/remote/remote_daemon.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupbackend.c | 3 +- src/util/vircgroupv1.c | 3 +- src/util/vircgroupv2.c | 3 +- src/util/virconf.c | 3 +- src/util/virenum.c | 28 ++- src/util/virenum.h | 15 +- src/util/virerror.c | 3 +- src/util/virfirewall.c | 3 +- src/util/virfirewalld.c | 6 +- src/util/virgic.c | 3 +- src/util/virhook.c | 20 +- src/util/virkeycode.c | 3 +- src/util/virlog.c | 3 +- src/util/virmdev.c | 3 +- src/util/virnetdev.c | 6 +- src/util/virnetdevmacvlan.c | 3 +- src/util/virnetdevvportprofile.c | 6 +- src/util/virpci.c | 9 +- src/util/virperf.c | 3 +- src/util/virprocess.c | 3 +- src/util/virresctrl.c | 12 +- src/util/virsecret.c | 3 +- src/util/virstorageencryption.c | 4 +- src/util/virstoragefile.c | 17 +- src/util/virsysinfo.c | 3 +- src/util/virtypedparam.c | 3 +- src/util/virutil.c | 1 - src/vmware/vmware_conf.c | 3 +- src/vmx/vmx.c | 3 +- tests/group-qemu-caps.pl | 2 +- tests/virkeycodetest.c | 3 +- tools/virsh-domain-monitor.c | 24 +-- tools/virsh-domain.c | 56 ++--- tools/virsh-host.c | 3 +- tools/virsh-network.c | 10 +- tools/virsh-nodedev.c | 2 +- tools/virsh-pool.c | 4 +- tools/virsh-secret.c | 2 +- tools/virsh-volume.c | 5 +- tools/virt-admin.c | 2 +- tools/virt-host-validate-common.c | 3 +- 77 files changed, 561 insertions(+), 334 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
