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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
