[libvirt] [PATCH 00/11] Replace virXXXFree calls with virObjectUnref

[John Ferlan]

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