On 12/01/2014 10:56 AM, John Ferlan wrote:
Based on some recent review comments and a bit of internal IRC, this
set of patches will change all the existing virXXXFree calls found in
daemon/* and src/* to be virObjectUnref instead and then add a rule to
inhibit usage unless the string/call is found in docs/*, tests/*, examples/*,
tools/*, cfg.mk, libvirt_public.syms, include/libvirt/libvirt-*.h, and
src/libvirt-*.c
Most of this was brute force to start with and adding the rule afterwards
caught a few oddball places.
The reason for not wanting to call a virXXXFree function is because it
will reset the last error, potentially clearing something and resulting
in a message "an error was encountered but the cause is unknown". There
were some places in the code which saved the last error, called the free
function, then restored the last error. Those have now been adjusted to
avoid that processing. Anything that required checking for a non NULL
pointer prior to calling the virXXXFree is now not necessary since the
virObjectUnref will check for NULL before continuing; whereas, the
virXXXFree functions didn't allow NULL.
If usage of the virXXXFree function is found during syntax-check, a message
such as the following will be displayed:
include/libvirt/libvirt-storage.h:256:int virStoragePoolFree
(virStoragePoolPtr pool);
include/libvirt/libvirt-storage.h:336:int virStorageVolFree
(virStorageVolPtr vol);
maint.mk: avoid using virXXXFree, use virObjectUnref instead
make: *** [sc_prohibit_virXXXFree] Error 1
John Ferlan (11):
rpc: Replace virXXXFree with virObjectUnref
Replace virDomainFree with virObjectUnref
Replace virNetworkFree with virObjectUnref
Replace virNodeDeviceFree with virObjectUnref
Replace virStorageVolFree with virObjectUnref
Replace virStoragePoolFree with virObjectUnref
Replace virStreamFree with virObjectUnref
Replace virSecretFree with virObjectUnref
Replace virNWFilterFree with virObjectUnref
Replace virInterfaceFree with virObjectUnref
Replace virDomainSnapshotFree with virObjectUnref
cfg.mk | 12 +++
daemon/remote.c | 168 ++++++++++++--------------------
daemon/stream.c | 2 +-
src/conf/domain_event.c | 4 +-
src/conf/network_conf.c | 6 +-
src/conf/network_event.c | 2 +-
src/conf/node_device_conf.c | 6 +-
src/conf/storage_conf.c | 6 +-
src/conf/virchrdev.c | 4 +-
src/esx/esx_driver.c | 2 +-
src/fdstream.c | 2 +-
src/hyperv/hyperv_driver.c | 2 +-
src/interface/interface_backend_netcf.c | 6 +-
src/interface/interface_backend_udev.c | 2 +-
src/libxl/libxl_conf.c | 7 +-
src/locking/sanlock_helper.c | 3 +-
src/lxc/lxc_driver.c | 8 +-
src/lxc/lxc_process.c | 8 +-
src/nwfilter/nwfilter_driver.c | 6 +-
src/qemu/qemu_command.c | 8 +-
src/qemu/qemu_driver.c | 4 +-
src/qemu/qemu_hotplug.c | 8 +-
src/remote/remote_driver.c | 76 +++++++--------
src/rpc/gendispatch.pl | 17 ++--
src/secret/secret_driver.c | 6 +-
src/storage/storage_backend.c | 4 +-
src/storage/storage_backend_fs.c | 4 +-
src/storage/storage_backend_rbd.c | 3 +-
src/storage/storage_driver.c | 20 +---
src/uml/uml_conf.c | 2 +-
src/vbox/vbox_common.c | 6 +-
src/xenconfig/xen_common.c | 2 +-
src/xenconfig/xen_sxpr.c | 2 +-
33 files changed, 155 insertions(+), 263 deletions(-)
Thanks for the review
- Laine: all set - rebase/merge picked that up and no thoughts to
backporting this as a whole!
- I adjusted patch 7 to remove the savedError logic and avoid the
extraneous intermediate variable
- I adjusted patch 10 to fix the commit message from "virInterfac3Free"
to be "virInterfaceFree"
and pushed (after making sure my Coverity build was happy too!)
John