Sigh.
Self-NACK to this entire series. I accidentally took the starting commit
ID for the starting patch in the git send-email command from a stale
instance of gitk that was sitting on my desktop, so these are *not* the
final versions or ordering of the patches :-(
I'm now sending the correct versions as "PATCH v2".
On 2/3/21 10:28 PM, Laine Stump wrote:
When I sent a patch to convert (what I thought was) all of the
VIR_FREEs in any *Free() function in the conf directory to g_free
(with the justification that it was trivial to verify that the
pointers weren't being referenced after they had been freed), there
were two responses that stuck in my mind:
1) Daniel said that this was a reasonable class of uses to change and
a reasonable way to arrive at the desired end result.
2) Peter justifiably expressing concern over the amount of churn in
the code, and also said that he'd rather not have things "halfway" for
an indeterminate time.
The combination of those two comments (ignoring the part about
"concern for churn" :-) let me to sit down last night and make this
patch series that eliminates all uses of VIR_FREE from any function
whose name ends in "Free" (or eliminate/rename the function, just for
completeness' sake)
In the vast majority of cases, this was done by replacing the
VIR_FREEs in the functions with g_free (while verifying that
everything being g_freed was actually something pointed to by the
parent object that was sent to the *Free() function, and that the
parent object itself was subsequently freed).
But there were a a couple of cases where this wasn't quite the right
thing to do:
* in patch 20, two functions ending with Free() don't actually free
the pointer they're sent. They instead behave like a *Clear()
function, VIR_FREEing their components. Since we can see that
these functions aren't actually be used as *Clear() functions (and
never will be used that way), but don't want to promote confusion
by leaving them misnamed, the patch renames them to *FreeContent()
(more accurate name) and changes their VIR_FREEs to g_frees (it
takes more than just looking at one function, but it's very easy
to verify that the pointers are never referenced after they're
freed).
* In Patch 21, a *Free() function actually appears to be used as a
*Clear() function, I left the VIR_FREEs in place, but renamed the
*function to *Clear(). However, its one use as a *Clear() is when
*being called from a "Reset" function that (as far as I can see)
*is never used. If we could get rid of that Reset function, I
*could just fold the *Clear() function into its one remaining
*caller, and eliminate the VIR_FREEs.
There is a companion series that eliminates VIR_FREE from all
*Dispose() functions, but I'm sending it separately since the two
series are completely independent.
The last time I ran wc over the diffs of these patches, it showed they
are eliminating 436 uses of VIR_FREE, which is > 10% of the 4k
remaining.
Laine Stump (21):
conf: replace remaining straggler VIR_FREE with g_free in vir*Free()
util: replace VIR_FREE with g_free in all vir*Free() functions
bhyve: replace VIR_FREE with g_free in all vir*Free() functions
libxl: replace VIR_FREE with g_free in all vir*Free() functions
qemu: replace VIR_FREE with g_free in all vir*Free() functions
test_driver: replace VIR_FREE with g_free in all vir*Free() functions
vbox: replace VIR_FREE with g_free in all vir*Free() functions
vmx: replace VIR_FREE with g_free in all vir*Free() functions
vz: replace VIR_FREE with g_free in all vir*Free() functions
admin: replace VIR_FREE with g_free in all vir*Free() functions
locking: replace VIR_FREE with g_free in all vir*Free() functions
logging: replace VIR_FREE with g_free in all vir*Free() functions
remote: replace VIR_FREE with g_free in all vir*Free() functions
rpc: replace VIR_FREE with g_free in all vir*Free() functions
security: replace VIR_FREE with g_free in all vir*Free() functions
tools: replace VIR_FREE with g_free in all vir*Free() functions
tests: replace VIR_FREE with g_free in all vir*Free() functions
storage: replace VIR_FREE with g_free in all vir*Free() functions
libvirtd: replace straggler VIR_FREE with g_free in all vir*Free()
functions
util: rename two *Free() functions while changing VIR_FREE to g_free
rpc: rename virNetSessionAuthMethodsFree to
virNetSessionAuthMethodsClear
src/admin/admin_server_dispatch.c | 2 +-
src/bhyve/bhyve_conf.c | 6 +-
src/bhyve/bhyve_domain.c | 2 +-
src/conf/domain_conf.c | 6 +-
src/conf/numa_conf.c | 10 +--
src/conf/storage_encryption_conf.c | 2 +-
src/conf/virchrdev.c | 6 +-
src/libvirt-domain.c | 18 +++---
src/libvirt-network.c | 14 ++---
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/libxl/libxl_migration.c | 6 +-
src/locking/lock_daemon.c | 6 +-
src/locking/lock_daemon_config.c | 6 +-
src/locking/lock_driver_lockd.c | 10 +--
src/locking/lock_driver_sanlock.c | 6 +-
src/locking/lock_manager.c | 2 +-
src/logging/log_daemon.c | 4 +-
src/logging/log_daemon_config.c | 6 +-
src/logging/log_handler.c | 6 +-
src/logging/log_manager.c | 2 +-
src/qemu/qemu_block.c | 36 +++++------
src/qemu/qemu_capabilities.c | 8 +--
src/qemu/qemu_cgroup.c | 4 +-
src/qemu/qemu_conf.c | 6 +-
src/qemu/qemu_domain.c | 14 ++---
src/qemu/qemu_firmware.c | 10 +--
src/qemu/qemu_migration_params.c | 4 +-
src/qemu/qemu_monitor.c | 48 +++++++--------
src/qemu/qemu_monitor_json.c | 22 +++----
src/qemu/qemu_process.c | 24 ++++----
src/qemu/qemu_saveimage.c | 6 +-
src/qemu/qemu_slirp.c | 2 +-
src/qemu/qemu_vhost_user.c | 8 +--
src/remote/remote_daemon_config.c | 48 +++++++--------
src/remote/remote_daemon_dispatch.c | 4 +-
src/remote/remote_driver.c | 2 +-
src/rpc/virnetmessage.c | 2 +-
src/rpc/virnetsshsession.c | 6 +-
src/security/security_dac.c | 8 +--
src/security/security_selinux.c | 10 +--
src/storage/storage_backend_fs.c | 6 +-
src/storage/storage_backend_rbd.c | 10 +--
src/storage/storage_backend_scsi.c | 4 +-
src/storage/storage_driver.c | 4 +-
src/test/test_driver.c | 6 +-
src/util/virarptable.c | 8 +--
src/util/virauthconfig.c | 4 +-
src/util/virbitmap.c | 4 +-
src/util/vircgroup.c | 12 ++--
src/util/vircommand.c | 24 ++++----
src/util/virconf.c | 10 +--
src/util/virdnsmasq.c | 32 +++++-----
src/util/virebtables.c | 4 +-
src/util/virfdstream.c | 10 +--
src/util/virfile.c | 4 +-
src/util/virfirewall.c | 16 ++---
src/util/virfirmware.c | 8 +--
src/util/virjson.c | 12 ++--
src/util/virlockspace.c | 12 ++--
src/util/virlog.c | 12 ++--
src/util/virmacaddr.c | 2 +-
src/util/virmdev.c | 16 ++---
src/util/virnetdev.c | 12 ++--
src/util/virnetdevbandwidth.c | 6 +-
src/util/virnetdevip.c | 6 +-
src/util/virnetdevmacvlan.c | 8 +--
src/util/virnetdevvlan.c | 2 +-
src/util/virnvme.c | 2 +-
src/util/virobject.c | 2 +-
src/util/virpci.c | 18 +++---
src/util/virperf.c | 2 +-
src/util/virportallocator.c | 4 +-
src/util/virresctrl.c | 6 +-
src/util/virrotatingfile.c | 14 ++---
src/util/virscsi.c | 16 ++---
src/util/virscsivhost.c | 10 +--
src/util/virseclabel.c | 16 ++---
src/util/virsocketaddr.c | 2 +-
src/util/virsysinfo.c | 96 ++++++++++++++---------------
src/util/virsystemd.c | 6 +-
src/util/virthreadpool.c | 6 +-
src/util/virtypedparam-public.c | 2 +-
src/util/virtypedparam.c | 8 +--
src/util/viruri.c | 20 +++---
src/util/virusb.c | 8 +--
src/util/virxml.c | 4 +-
src/vbox/vbox_snapshot_conf.c | 54 ++++++++--------
src/vmx/vmx.c | 6 +-
src/vz/vz_driver.c | 8 +--
src/vz/vz_utils.c | 2 +-
tests/nodedevmdevctltest.c | 4 +-
tests/nwfilterxml2firewalltest.c | 2 +-
tests/qemuhotplugtest.c | 12 ++--
tests/qemumonitortestutils.c | 28 ++++-----
tests/virnetdaemontest.c | 2 +-
tests/virnetserverclienttest.c | 2 +-
tools/virsh-checkpoint.c | 6 +-
tools/virsh-domain-monitor.c | 4 +-
tools/virsh-domain.c | 2 +-
tools/virsh-interface.c | 4 +-
tools/virsh-network.c | 8 +--
tools/virsh-nodedev.c | 4 +-
tools/virsh-nwfilter.c | 8 +--
tools/virsh-pool.c | 4 +-
tools/virsh-secret.c | 4 +-
tools/virsh-snapshot.c | 6 +-
tools/virsh-volume.c | 4 +-
tools/vsh-table.c | 10 +--
tools/vsh.c | 6 +-
110 files changed, 541 insertions(+), 541 deletions(-)
