On 2/4/21 1:38 AM, Laine Stump wrote:
> A *Dispose() function is similar to a *Free() function, except that 1)
> the object is always sent as a void* so it has to be typecast into
> some other object-specific pointer at the top of the function, and 2)
> it frees all the resources inside the object, but never frees the
> object itself (this is done by the caller, somewhere deep in the
> bowels of virObjectDispose or something I guess; frankly I've always
> ignored the details simply "because I could").
>
> The important point is that the contents of the object are never
> referenced in any way after return from the Dispose function, so it is
> unnecessary to clear any pointers, ergo (always wanted to use that
> word!) it's completely safe to replace all VIR_FREEs in a *Dispose()
> function with g_free (as long as there's nothing within the Dispose
> function itself that depends on the pointers being cleared).
>
> After this series is applied, there will be exactly 0 instances of
> VIR_FREE in any *Dispose() (or *Free()) function. As with the *Free()
> series, almost all were accomplished by directly replacing VIR_FREE
> with g_free, but there were a couple oddities that needed separate
> patches just to call them out:
>
> * Patch 1 & 2 - in both cases a Dispose() function was the only
> caller to a *Free() function that didn't fit the normal pattern of
> a *Free() function. Since each of the Free()s had only one caller
> (their respective *Dispose() parents), their contents were just
> moved into the caller, clearing the way for their VIR_FREEs to be
> g_free-ified (in later patches, along with the other *Dispose()
> functions in the same directories).
>
> 220 VIR_FREE uses eliminated in this series, so a total of 762 for the
> two series combined (nearly 20% of all remaining VIR_FREEs).
>
> Laine Stump (15):
> conf: simplify virDomainCapsDispose()
> rpc: eliminate static function virNetLibsshSessionAuthMethodsFree()
> bhyve: replace VIR_FREE with g_free in all *Dispose() functions
> libxl: replace VIR_FREE with g_free in all *Dispose() functions
> qemu: replace VIR_FREE with g_free in all *Dispose() functions
> interface: replace VIR_FREE with g_free in all *Dispose() functions
> access: replace VIR_FREE with g_free in all *Dispose() functions
> hypervisor: replace VIR_FREE with g_free in all *Dispose() functions
> logging: replace VIR_FREE with g_free in all *Dispose() functions
> rpc: replace VIR_FREE with g_free in all *Dispose() functions
> security: replace VIR_FREE with g_free in all *Dispose() functions
> util: replace VIR_FREE with g_free in all *Dispose() functions
> conf: replace VIR_FREE with g_free in all *Dispose() functions
> tests: replace VIR_FREE with g_free in all *Dispose() functions
> datatypes: replace VIR_FREE with g_free in all *Dispose() functions
Series LGTM, just be careful with what I mentioned in patch 10.
All patches:
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>
> src/access/viraccessmanager.c | 2 +-
> src/bhyve/bhyve_conf.c | 2 +-
> src/conf/capabilities.c | 22 ++---
> src/conf/checkpoint_conf.c | 2 +-
> src/conf/domain_capabilities.c | 29 ++----
> src/conf/domain_conf.c | 2 +-
> src/conf/domain_event.c | 52 +++++-----
> src/conf/moment_conf.c | 6 +-
> src/conf/object_event.c | 4 +-
> src/conf/snapshot_conf.c | 4 +-
> src/conf/virsecretobj.c | 6 +-
> src/conf/virstorageobj.c | 4 +-
> src/datatypes.c | 34 +++----
> src/hypervisor/virhostdev.c | 2 +-
> src/interface/interface_backend_netcf.c | 2 +-
> src/libxl/libxl_conf.c | 20 ++--
> src/libxl/libxl_migration.c | 2 +-
> src/logging/log_handler.c | 2 +-
> src/qemu/qemu_agent.c | 2 +-
> src/qemu/qemu_capabilities.c | 10 +-
> src/qemu/qemu_conf.c | 122 ++++++++++++------------
> src/qemu/qemu_domain.c | 10 +-
> src/qemu/qemu_monitor.c | 4 +-
> src/rpc/virnetclient.c | 4 +-
> src/rpc/virnetdaemon.c | 6 +-
> src/rpc/virnetlibsshsession.c | 37 +++----
> src/rpc/virnetsaslcontext.c | 2 +-
> src/rpc/virnetserver.c | 8 +-
> src/rpc/virnetserverservice.c | 2 +-
> src/rpc/virnetsocket.c | 6 +-
> src/rpc/virnetsshsession.c | 8 +-
> src/rpc/virnettlscontext.c | 6 +-
> src/security/security_manager.c | 2 +-
> src/util/virdnsmasq.c | 2 +-
> src/util/virfilecache.c | 4 +-
> src/util/virmdev.c | 2 +-
> src/util/virnvme.c | 2 +-
> src/util/virpci.c | 2 +-
> src/util/virresctrl.c | 40 ++++----
> src/util/virscsi.c | 2 +-
> src/util/virscsivhost.c | 2 +-
> src/util/virusb.c | 2 +-
> tests/virfilecachetest.c | 2 +-
> 43 files changed, 235 insertions(+), 251 deletions(-)
>