12:27 a.m.
Another set of small changes that started out with me noticing
something I didn't like, and organically leading down a Kerouacian
road to nowhere and everywhere.
Most of them are related to eliminating unnecessary nullifying of
objects that are about to be free'd anyway, but patch 2 is obsolete
dead code, and patch 1 is a theoretical "failure with no registered
error message".
Laine Stump (8):
log error if virConnectCacheOnceInit() fails
hostdevmgr: remove unneeded oldStateDir
conf: replace g_clear_pointer(..., g_hash_table_unref) with
virHashFree()
conf: fix arg to virDomainPCIAddressSetExtensionFree()
conf: don't bother setting pointers to NULL in vir*Free() functions
conf: eliminate pointless setting of interface model
util: rename virStorageEncryptionInfoDefFree()
conf: replace VIR_FREE() with g_free() in vir*Free() functions
src/conf/capabilities.c | 8 +-
src/conf/cpu_conf.c | 8 +-
src/conf/device_conf.c | 2 +-
src/conf/domain_addr.c | 32 +--
src/conf/domain_capabilities.c | 6 +-
src/conf/domain_conf.c | 304 +++++++++++++-------------
src/conf/interface_conf.c | 32 +--
src/conf/network_conf.c | 18 +-
src/conf/node_device_conf.c | 105 +++++----
src/conf/nwfilter_conf.c | 22 +-
src/conf/nwfilter_params.c | 16 +-
src/conf/object_event.c | 12 +-
src/conf/secret_conf.c | 6 +-
src/conf/snapshot_conf.c | 2 +-
src/conf/storage_conf.c | 22 +-
src/conf/storage_encryption_conf.c | 8 +-
src/conf/storage_source_conf.c | 26 +--
src/conf/virdomaincheckpointobjlist.c | 2 +-
src/conf/virdomainmomentobjlist.c | 4 +-
src/conf/virdomainsnapshotobjlist.c | 2 +-
src/conf/virnetworkportdef.c | 12 +-
src/conf/virnwfilterbindingdef.c | 10 +-
src/conf/virnwfilterobj.c | 6 +-
src/driver.c | 22 +-
src/hypervisor/virhostdev.c | 60 +----
src/hypervisor/virhostdev.h | 6 +-
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 4 +-
src/qemu/qemu_hostdev.c | 3 +-
tests/virhostdevtest.c | 10 +-
30 files changed, 363 insertions(+), 409 deletions(-)
--
2.29.2
12:27 a.m.
New subject: [PATCH 1/8] log error if virConnectCacheOnceInit() fails
This may be pointless, but I noticed it and it was bugging me.
virGetConnectNetwork() calls
virGetConnectGeneric(), which calls
virConnecCacheInitialize(), which is actually a call (only once) to
virConnectCacheOnceInit() which calls
virThreadLocalInit() several times, which calls
pthread_key_create()
If pthread_key_create() fails, it (of course) doesn't log an error
(because it's not a part of libvirt), nor does any other function on
the call chain all the way up to virGetConnectNetwork(). But none of
the callers of virGetConnectNetwork() log an error either, so it is
possible that an API could fail due to virGetConnectNetwork() failing,
but would only log "an error was encountered, but the cause is
unknown. Deal with it." (paraphrasing).
In all likelyhood, virConnectCacheOnceInit() is going to be called at
some earlier time, and almost certainly pthread_key_create() will
never fail (and if it does, the user will have *much* bigger problems
than an obtuse error message from libvirt). So I don't know if there's
any value at all in pushing this. Just throwing it out there...
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/driver.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/driver.c b/src/driver.c
index e005a89d57..1dae7cf284 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -116,18 +116,18 @@ virThreadLocal connectStorage;
static int
virConnectCacheOnceInit(void)
{
- if (virThreadLocalInit(&connectInterface, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNetwork, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNWFilter, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNodeDev, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectSecret, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectStorage, NULL) < 0)
+ if (virThreadLocalInit(&connectInterface, NULL) < 0
+ || virThreadLocalInit(&connectNetwork, NULL) < 0
+ || virThreadLocalInit(&connectNWFilter, NULL) < 0
+ || virThreadLocalInit(&connectNodeDev, NULL) < 0
+ || virThreadLocalInit(&connectSecret, NULL) < 0
+ || virThreadLocalInit(&connectStorage, NULL) < 0) {
+
+ virReportSystemError(errno, "%s",
+ _("Unable to initialize thread local variable"));
return -1;
+ }
+
return 0;
}
--
2.29.2
3:18 a.m.
New subject: [PATCH 1/8] log error if virConnectCacheOnceInit() fails
On 2/1/21 7:27 AM, Laine Stump wrote:
This may be pointless, but I noticed it and it was bugging me.
virGetConnectNetwork() calls
virGetConnectGeneric(), which calls
virConnecCacheInitialize(), which is actually a call (only once) to
virConnectCacheOnceInit() which calls
virThreadLocalInit() several times, which calls
pthread_key_create()
If pthread_key_create() fails, it (of course) doesn't log an error
(because it's not a part of libvirt), nor does any other function on
the call chain all the way up to virGetConnectNetwork(). But none of
the callers of virGetConnectNetwork() log an error either, so it is
possible that an API could fail due to virGetConnectNetwork() failing,
but would only log "an error was encountered, but the cause is
unknown. Deal with it." (paraphrasing).
In all likelyhood, virConnectCacheOnceInit() is going to be called at
some earlier time, and almost certainly pthread_key_create() will
never fail (and if it does, the user will have *much* bigger problems
than an obtuse error message from libvirt). So I don't know if there's
any value at all in pushing this. Just throwing it out there...
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/driver.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/driver.c b/src/driver.c
index e005a89d57..1dae7cf284 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -116,18 +116,18 @@ virThreadLocal connectStorage;
static int
virConnectCacheOnceInit(void)
{
- if (virThreadLocalInit(&connectInterface, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNetwork, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNWFilter, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNodeDev, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectSecret, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectStorage, NULL) < 0)
+ if (virThreadLocalInit(&connectInterface, NULL) < 0
+ || virThreadLocalInit(&connectNetwork, NULL) < 0
+ || virThreadLocalInit(&connectNWFilter, NULL) < 0
+ || virThreadLocalInit(&connectNodeDev, NULL) < 0
+ || virThreadLocalInit(&connectSecret, NULL) < 0
+ || virThreadLocalInit(&connectStorage, NULL) < 0) {
I think our preferred way of breaking these checks is to put '||' at the
end of each line rather than at the beginning. It it isn't then it
should be :-)
+
Maybe drop this empty line?
> + virReportSystemError(errno, "%s",
> + _("Unable to initialize thread local
variable"));
> return -1;
> + }
+
> return 0;
> }
>
>
Michal
1:50 p.m.
New subject: [PATCH 1/8] log error if virConnectCacheOnceInit() fails
On 2/1/21 4:18 AM, Michal Privoznik wrote:
On 2/1/21 7:27 AM, Laine Stump wrote:
> This may be pointless, but I noticed it and it was bugging me.
>
> virGetConnectNetwork() calls
> virGetConnectGeneric(), which calls
> virConnecCacheInitialize(), which is actually a call (only once) to
> virConnectCacheOnceInit() which calls
> virThreadLocalInit() several times, which calls
> pthread_key_create()
>
> If pthread_key_create() fails, it (of course) doesn't log an error
> (because it's not a part of libvirt), nor does any other function on
> the call chain all the way up to virGetConnectNetwork(). But none of
> the callers of virGetConnectNetwork() log an error either, so it is
> possible that an API could fail due to virGetConnectNetwork() failing,
> but would only log "an error was encountered, but the cause is
> unknown. Deal with it." (paraphrasing).
>
> In all likelyhood, virConnectCacheOnceInit() is going to be called at
> some earlier time, and almost certainly pthread_key_create() will
> never fail (and if it does, the user will have *much* bigger problems
> than an obtuse error message from libvirt). So I don't know if there's
> any value at all in pushing this. Just throwing it out there...
>
> Signed-off-by: Laine Stump <laine(a)redhat.com>
> ---
> src/driver.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/src/driver.c b/src/driver.c
> index e005a89d57..1dae7cf284 100644
> --- a/src/driver.c
> +++ b/src/driver.c
> @@ -116,18 +116,18 @@ virThreadLocal connectStorage;
> static int
> virConnectCacheOnceInit(void)
> {
> - if (virThreadLocalInit(&connectInterface, NULL) < 0)
> - return -1;
> - if (virThreadLocalInit(&connectNetwork, NULL) < 0)
> - return -1;
> - if (virThreadLocalInit(&connectNWFilter, NULL) < 0)
> - return -1;
> - if (virThreadLocalInit(&connectNodeDev, NULL) < 0)
> - return -1;
> - if (virThreadLocalInit(&connectSecret, NULL) < 0)
> - return -1;
> - if (virThreadLocalInit(&connectStorage, NULL) < 0)
> + if (virThreadLocalInit(&connectInterface, NULL) < 0
> + || virThreadLocalInit(&connectNetwork, NULL) < 0
> + || virThreadLocalInit(&connectNWFilter, NULL) < 0
> + || virThreadLocalInit(&connectNodeDev, NULL) < 0
> + || virThreadLocalInit(&connectSecret, NULL) < 0
> + || virThreadLocalInit(&connectStorage, NULL) < 0) {
I think our preferred way of breaking these checks is to put '||' at
the end of each line rather than at the beginning. It it isn't then it
should be :-)
You're correct. Sometimes my brain shifts back to 1995 mode, when I
worked with people who preferred it as above. I'll switch it.
> +
Maybe drop this empty line?
Sure.
> + virReportSystemError(errno, "%s",
> + _("Unable to initialize thread local
> variable"));
> return -1;
> + }
> +
> return 0;
> }
>
Michal
12:27 a.m.
New subject: [PATCH 2/8] hostdevmgr: remove unneeded oldStateDir
Back in commit 2c71d3826, which appeared in libvirt-1.2.3 in April
2014, the location used to store saved MAC addresses and vlan tags of
SRIOV VFs was changed from /var/run/libvirt/qemu to
/var/run/libvirt/hostdevmgr. For backward compatibility the code was
made to continue looking in the old location for the files when it
didn't find them in the new location.
It's now been 6 years, and even if there was somebody still running
libvirt-1.2.3 on their system, that system would now be out of support
for libvirt, so there would be no way for them to upgrade to a new
libvirt that no longer looks in "oldStateDir" for the files. So
let's no longer look in "oldStateDir" for the files!
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/hypervisor/virhostdev.c | 60 ++++++++-----------------------------
src/hypervisor/virhostdev.h | 6 ++--
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 4 +--
src/qemu/qemu_hostdev.c | 3 +-
tests/virhostdevtest.c | 10 +++----
6 files changed, 23 insertions(+), 62 deletions(-)
diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index bd35397f2c..09995a52ed 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -484,17 +484,9 @@ virHostdevSetNetConfig(virDomainHostdevDefPtr hostdev,
}
-/* @oldStateDir:
- * For upgrade purpose:
- * To an existing VM on QEMU, the hostdev netconfig file is originally stored
- * in cfg->stateDir (/var/run/libvirt/qemu). Switch to new version, it uses new
- * location (mgr->stateDir) but certainly will not find it. In this
- * case, try to find in the old state dir.
- */
static int
virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
- const char *stateDir,
- const char *oldStateDir)
+ const char *stateDir)
{
g_autofree char *linkdev = NULL;
g_autofree virMacAddrPtr MAC = NULL;
@@ -528,16 +520,11 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
NULL,
port_profile_associate);
} else {
- /* we need to try 3 different places for the config file:
+ /* we need to try 2 different places for the config file:
* 1) ${stateDir}/${PF}_vf${vf}
* This is almost always where the saved config is
*
- * 2) ${oldStateDir/${PF}_vf${vf}
- * saved config is only here if this machine was running a
- * (by now *very*) old version of libvirt that saved the
- * file in a different directory
- *
- * 3) ${stateDir}${PF[1]}_vf${VF}
+ * 2) ${stateDir}${PF[1]}_vf${VF}
* PF[1] means "the netdev for port 2 of the PF device", and
* is only valid when the PF is a Mellanox dual port NIC with
* a VF that was created in "single port" mode.
@@ -556,18 +543,7 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
return -1;
}
- /* 2) "old" (pre-1.2.3 circa 2014) location - whenever we get
- * to the point that nobody will ever upgrade directly from
- * 1.2.3 (or older) directly to current libvirt, we can
- * eliminate this clause
- **/
- if (!(adminMAC || vlan || MAC) && oldStateDir &&
- virNetDevReadNetConfig(linkdev, vf, oldStateDir,
- &adminMAC, &vlan, &MAC) < 0) {
- return -1;
- }
-
- /* 3) try using the PF's "port 2" netdev as the name of the
+ /* 2) try using the PF's "port 2" netdev as the name of the
* config file
*/
if (!(adminMAC || vlan || MAC)) {
@@ -918,7 +894,7 @@ virHostdevPreparePCIDevicesImpl(virHostdevManagerPtr mgr,
resetvfnetconfig:
if (last_processed_hostdev_vf >= 0) {
for (i = 0; i <= last_processed_hostdev_vf; i++)
- virHostdevRestoreNetConfig(hostdevs[i], mgr->stateDir, NULL);
+ virHostdevRestoreNetConfig(hostdevs[i], mgr->stateDir);
}
reattachdevs:
@@ -960,8 +936,7 @@ virHostdevReAttachPCIDevicesImpl(virHostdevManagerPtr mgr,
const char *dom_name,
virPCIDeviceListPtr pcidevs,
virDomainHostdevDefPtr *hostdevs,
- int nhostdevs,
- const char *oldStateDir)
+ int nhostdevs)
{
size_t i;
@@ -1043,8 +1018,7 @@ virHostdevReAttachPCIDevicesImpl(virHostdevManagerPtr mgr,
if (actual) {
VIR_DEBUG("Restoring network configuration of PCI device %s",
virPCIDeviceGetName(actual));
- virHostdevRestoreNetConfig(hostdev, mgr->stateDir,
- oldStateDir);
+ virHostdevRestoreNetConfig(hostdev, mgr->stateDir);
}
}
}
@@ -1061,16 +1035,12 @@ virHostdevReAttachPCIDevicesImpl(virHostdevManagerPtr mgr,
}
-/* @oldStateDir:
- * For upgrade purpose: see virHostdevRestoreNetConfig
- */
void
virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
const char *drv_name,
const char *dom_name,
virDomainHostdevDefPtr *hostdevs,
- int nhostdevs,
- const char *oldStateDir)
+ int nhostdevs)
{
g_autoptr(virPCIDeviceList) pcidevs = NULL;
@@ -1085,7 +1055,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr,
}
virHostdevReAttachPCIDevicesImpl(mgr, drv_name, dom_name, pcidevs,
- hostdevs, nhostdevs, oldStateDir);
+ hostdevs, nhostdevs);
}
@@ -2120,23 +2090,18 @@ virHostdevPrepareDomainDevices(virHostdevManagerPtr mgr,
return 0;
}
-/* @oldStateDir
- * For upgrade purpose: see virHostdevReAttachPCIHostdevs
- */
void
virHostdevReAttachDomainDevices(virHostdevManagerPtr mgr,
const char *driver,
virDomainDefPtr def,
- unsigned int flags,
- const char *oldStateDir)
+ unsigned int flags)
{
if (!def->nhostdevs || !mgr)
return;
if (flags & VIR_HOSTDEV_SP_PCI) {
virHostdevReAttachPCIDevices(mgr, driver, def->name,
- def->hostdevs, def->nhostdevs,
- oldStateDir);
+ def->hostdevs, def->nhostdevs);
}
if (flags & VIR_HOSTDEV_SP_USB) {
@@ -2388,8 +2353,7 @@ virHostdevReAttachOneNVMeDevice(virHostdevManagerPtr hostdev_mgr,
goto cleanup;
virHostdevReAttachPCIDevicesImpl(hostdev_mgr,
- drv_name, dom_name, pciDevices,
- NULL, 0, NULL);
+ drv_name, dom_name, pciDevices, NULL, 0);
for (i = 0; i < virNVMeDeviceListCount(nvmeDevices); i++) {
virNVMeDevicePtr temp = virNVMeDeviceListGet(nvmeDevices, i);
diff --git a/src/hypervisor/virhostdev.h b/src/hypervisor/virhostdev.h
index 811bda40ed..19e1938d9e 100644
--- a/src/hypervisor/virhostdev.h
+++ b/src/hypervisor/virhostdev.h
@@ -113,8 +113,7 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr hostdev_mgr,
const char *drv_name,
const char *dom_name,
virDomainHostdevDefPtr *hostdevs,
- int nhostdevs,
- const char *oldStateDir)
+ int nhostdevs)
ATTRIBUTE_NONNULL(1);
void
virHostdevReAttachUSBDevices(virHostdevManagerPtr hostdev_mgr,
@@ -188,8 +187,7 @@ void
virHostdevReAttachDomainDevices(virHostdevManagerPtr mgr,
const char *driver,
virDomainDefPtr def,
- unsigned int flags,
- const char *oldStateDir)
+ unsigned int flags)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
/* functions used by NodeDevDetach/Reattach/Reset */
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index afa21bf02e..380f7e0b56 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -868,7 +868,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver,
}
virHostdevReAttachDomainDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
- vm->def, hostdev_flags, NULL);
+ vm->def, hostdev_flags);
VIR_FREE(priv->lockState);
if (virDomainLockProcessPause(driver->lockManager, vm, &priv->lockState)
< 0)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 3eaf106006..f480f8067e 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3153,7 +3153,7 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver,
error:
virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
- vm->def->name, &hostdev, 1, NULL);
+ vm->def->name, &hostdev, 1);
cleanup:
virObjectUnref(cfg);
@@ -3690,7 +3690,7 @@ libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver,
virDomainHostdevRemove(vm->def, idx);
virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_INTERNAL_NAME,
- vm->def->name, &hostdev, 1, NULL);
+ vm->def->name, &hostdev, 1);
ret = 0;
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 721fe5da82..57971214b7 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -406,11 +406,10 @@ qemuHostdevReAttachPCIDevices(virQEMUDriverPtr driver,
int nhostdevs)
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
- const char *oldStateDir = cfg->stateDir;
virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
virHostdevReAttachPCIDevices(hostdev_mgr, QEMU_DRIVER_NAME, name,
- hostdevs, nhostdevs, oldStateDir);
+ hostdevs, nhostdevs);
}
void
diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 40c14a5281..91f9112e8b 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -245,13 +245,13 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(void)
inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
VIR_TEST_DEBUG("Test 0 hostdevs");
- virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, NULL, 0, NULL);
+ virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, NULL, 0);
CHECK_PCI_LIST_COUNT(mgr->activePCIHostdevs, active_count);
CHECK_PCI_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
VIR_TEST_DEBUG("Test >=1 unmanaged hostdevs");
virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
- hostdevs, nhostdevs, NULL);
+ hostdevs, nhostdevs);
CHECK_PCI_LIST_COUNT(mgr->activePCIHostdevs, active_count - nhostdevs);
CHECK_PCI_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count + nhostdevs);
@@ -329,13 +329,13 @@ testVirHostdevReAttachPCIHostdevs_managed(bool mixed)
inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
VIR_TEST_DEBUG("Test 0 hostdevs");
- virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, NULL, 0, NULL);
+ virHostdevReAttachPCIDevices(mgr, drv_name, dom_name, NULL, 0);
CHECK_PCI_LIST_COUNT(mgr->activePCIHostdevs, active_count);
CHECK_PCI_LIST_COUNT(mgr->inactivePCIHostdevs, inactive_count);
VIR_TEST_DEBUG("Test >=1 hostdevs");
virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
- hostdevs, nhostdevs, NULL);
+ hostdevs, nhostdevs);
CHECK_PCI_LIST_COUNT(mgr->activePCIHostdevs, active_count - nhostdevs);
/* If testing a mixed roundtrip, devices are added back to the inactive
* list as soon as we detach from the guest */
@@ -542,7 +542,7 @@ testNVMeDiskRoundtrip(const void *opaque G_GNUC_UNUSED)
/* Don't rely on a state that previous test cases might have
* left the manager in. Start with a clean slate. */
virHostdevReAttachPCIDevices(mgr, drv_name, dom_name,
- hostdevs, nhostdevs, NULL);
+ hostdevs, nhostdevs);
CHECK_NVME_LIST_COUNT(mgr->activeNVMeHostdevs, 0);
CHECK_PCI_LIST_COUNT(mgr->activePCIHostdevs, 0);
--
2.29.2
12:27 a.m.
New subject: [PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
virHashFree() just calls g_hash_table_unref(), and it's more common
for libvirt code to call virHashFree() rather than the convoluted
calling of g_hash_table_unref() via g_clear_pointer().
Since the object containing the hashes is g_freed immediately after
the hashes are freed, there is no functional difference.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/domain_addr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 37dad20ade..a8648d5858 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -949,8 +949,8 @@ virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
if (!addrs || !addrs->zpciIds)
return;
- g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref);
- g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref);
+ virHashFree(addrs->zpciIds->uids);
+ virHashFree(addrs->zpciIds->fids);
VIR_FREE(addrs->zpciIds);
}
--
2.29.2
3:18 a.m.
New subject: [PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
On 2/1/21 7:27 AM, Laine Stump wrote:
virHashFree() just calls g_hash_table_unref(), and it's more
common
for libvirt code to call virHashFree() rather than the convoluted
calling of g_hash_table_unref() via g_clear_pointer().
Since the object containing the hashes is g_freed immediately after
the hashes are freed, there is no functional difference.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/domain_addr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 37dad20ade..a8648d5858 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -949,8 +949,8 @@ virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
if (!addrs || !addrs->zpciIds)
return;
- g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref);
- g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref);
+ virHashFree(addrs->zpciIds->uids);
+ virHashFree(addrs->zpciIds->fids);
VIR_FREE(addrs->zpciIds);
}
virHashFree documents itself as being deprecated in favor of
g_hash_table_unref().
While I like our virSomething wrappers (mostly because I'm used to them
more than to their glib counterparts; but then you also have glib
functions when one thinks that glib implementation is interchangeable
with ours but it isn't - devil's in the details), I think our intent is
to drop virHashFree().
But then again - we didn't, instead we replaced virHash* internals with
glib, so I can argue that being consistent is more important than being
progressive.
Your call, but since you build next patch on this one, I'm inclined to
say it's okay to merge it.
Michal
3:47 a.m.
New subject: [PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
On Mon, Feb 01, 2021 at 10:18:52 +0100, Michal Privoznik wrote:
On 2/1/21 7:27 AM, Laine Stump wrote:
> virHashFree() just calls g_hash_table_unref(), and it's more common
> for libvirt code to call virHashFree() rather than the convoluted
> calling of g_hash_table_unref() via g_clear_pointer().
>
> Since the object containing the hashes is g_freed immediately after
> the hashes are freed, there is no functional difference.
>
> Signed-off-by: Laine Stump <laine(a)redhat.com>
> ---
> src/conf/domain_addr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 37dad20ade..a8648d5858 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -949,8 +949,8 @@ virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr
addrs)
> if (!addrs || !addrs->zpciIds)
> return;
> - g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref);
> - g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref);
> + virHashFree(addrs->zpciIds->uids);
> + virHashFree(addrs->zpciIds->fids);
> VIR_FREE(addrs->zpciIds);
> }
>
virHashFree documents itself as being deprecated in favor of
g_hash_table_unref().
While I like our virSomething wrappers (mostly because I'm used to them more
than to their glib counterparts; but then you also have glib functions when
one thinks that glib implementation is interchangeable with ours but it
isn't - devil's in the details), I think our intent is to drop
virHashFree().
But then again - we didn't, instead we replaced virHash* internals with
glib, so I can argue that being consistent is more important than being
progressive.
Your call, but since you build next patch on this one, I'm inclined to say
it's okay to merge it.
It's a NACK from me. That was deliberate. Especially virHashFree doesn't
clear the pointer, the code which we have does.
3:50 a.m.
New subject: [PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
On 2/1/21 10:47 AM, Peter Krempa wrote:
On Mon, Feb 01, 2021 at 10:18:52 +0100, Michal Privoznik wrote:
> On 2/1/21 7:27 AM, Laine Stump wrote:
>> virHashFree() just calls g_hash_table_unref(), and it's more common
>> for libvirt code to call virHashFree() rather than the convoluted
>> calling of g_hash_table_unref() via g_clear_pointer().
>>
>> Since the object containing the hashes is g_freed immediately after
>> the hashes are freed, there is no functional difference.
>>
>> Signed-off-by: Laine Stump <laine(a)redhat.com>
>> ---
>> src/conf/domain_addr.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
>> index 37dad20ade..a8648d5858 100644
>> --- a/src/conf/domain_addr.c
>> +++ b/src/conf/domain_addr.c
>> @@ -949,8 +949,8 @@ virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr
addrs)
>> if (!addrs || !addrs->zpciIds)
>> return;
>> - g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref);
>> - g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref);
>> + virHashFree(addrs->zpciIds->uids);
>> + virHashFree(addrs->zpciIds->fids);
>> VIR_FREE(addrs->zpciIds);
>> }
>>
>
> virHashFree documents itself as being deprecated in favor of
> g_hash_table_unref().
>
> While I like our virSomething wrappers (mostly because I'm used to them more
> than to their glib counterparts; but then you also have glib functions when
> one thinks that glib implementation is interchangeable with ours but it
> isn't - devil's in the details), I think our intent is to drop
> virHashFree().
>
> But then again - we didn't, instead we replaced virHash* internals with
> glib, so I can argue that being consistent is more important than being
> progressive.
>
> Your call, but since you build next patch on this one, I'm inclined to say
> it's okay to merge it.
It's a NACK from me. That was deliberate. Especially virHashFree doesn't
clear the pointer, the code which we have does.
But as can be seen from the context, the whole object is freed
immediately afterwards. IOW, this is what's happening:
free(obj->ptr);
obj->ptr = NULL;
free(obj);
Is the pointer clearing necessary?
Michal
3:56 a.m.
New subject: [PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
On Mon, Feb 01, 2021 at 10:50:47 +0100, Michal Privoznik wrote:
On 2/1/21 10:47 AM, Peter Krempa wrote:
> On Mon, Feb 01, 2021 at 10:18:52 +0100, Michal Privoznik wrote:
> > On 2/1/21 7:27 AM, Laine Stump wrote:
> > > virHashFree() just calls g_hash_table_unref(), and it's more common
> > > for libvirt code to call virHashFree() rather than the convoluted
> > > calling of g_hash_table_unref() via g_clear_pointer().
> > >
> > > Since the object containing the hashes is g_freed immediately after
> > > the hashes are freed, there is no functional difference.
> > >
> > > Signed-off-by: Laine Stump <laine(a)redhat.com>
> > > ---
> > > src/conf/domain_addr.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> > > index 37dad20ade..a8648d5858 100644
> > > --- a/src/conf/domain_addr.c
> > > +++ b/src/conf/domain_addr.c
> > > @@ -949,8 +949,8 @@
virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
> > > if (!addrs || !addrs->zpciIds)
> > > return;
> > > - g_clear_pointer(&addrs->zpciIds->uids,
g_hash_table_unref);
> > > - g_clear_pointer(&addrs->zpciIds->fids,
g_hash_table_unref);
> > > + virHashFree(addrs->zpciIds->uids);
> > > + virHashFree(addrs->zpciIds->fids);
> > > VIR_FREE(addrs->zpciIds);
> > > }
> > >
> >
> > virHashFree documents itself as being deprecated in favor of
> > g_hash_table_unref().
> >
> > While I like our virSomething wrappers (mostly because I'm used to them
more
> > than to their glib counterparts; but then you also have glib functions when
> > one thinks that glib implementation is interchangeable with ours but it
> > isn't - devil's in the details), I think our intent is to drop
> > virHashFree().
> >
> > But then again - we didn't, instead we replaced virHash* internals with
> > glib, so I can argue that being consistent is more important than being
> > progressive.
> >
> > Your call, but since you build next patch on this one, I'm inclined to say
> > it's okay to merge it.
>
> It's a NACK from me. That was deliberate. Especially virHashFree doesn't
> clear the pointer, the code which we have does.
>
But as can be seen from the context, the whole object is freed immediately
afterwards. IOW, this is what's happening:
free(obj->ptr);
obj->ptr = NULL;
free(obj);
Is the pointer clearing necessary?
Not really, but g_hash_table_unref doesn't tolerate NULL argument, while
using g_clear_pointer makes it tolerate it, thus the code is a bit
neater IMO than an explicit if (ptr).
3:51 a.m.
New subject: [PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
On Mon, Feb 01, 2021 at 10:47:47 +0100, Peter Krempa wrote:
On Mon, Feb 01, 2021 at 10:18:52 +0100, Michal Privoznik wrote:
> On 2/1/21 7:27 AM, Laine Stump wrote:
> > virHashFree() just calls g_hash_table_unref(), and it's more common
> > for libvirt code to call virHashFree() rather than the convoluted
> > calling of g_hash_table_unref() via g_clear_pointer().
> >
> > Since the object containing the hashes is g_freed immediately after
> > the hashes are freed, there is no functional difference.
> >
> > Signed-off-by: Laine Stump <laine(a)redhat.com>
> > ---
> > src/conf/domain_addr.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> > index 37dad20ade..a8648d5858 100644
> > --- a/src/conf/domain_addr.c
> > +++ b/src/conf/domain_addr.c
> > @@ -949,8 +949,8 @@
virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
> > if (!addrs || !addrs->zpciIds)
> > return;
> > - g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref);
> > - g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref);
> > + virHashFree(addrs->zpciIds->uids);
> > + virHashFree(addrs->zpciIds->fids);
> > VIR_FREE(addrs->zpciIds);
> > }
> >
>
> virHashFree documents itself as being deprecated in favor of
> g_hash_table_unref().
>
> While I like our virSomething wrappers (mostly because I'm used to them more
> than to their glib counterparts; but then you also have glib functions when
> one thinks that glib implementation is interchangeable with ours but it
> isn't - devil's in the details), I think our intent is to drop
> virHashFree().
>
> But then again - we didn't, instead we replaced virHash* internals with
> glib, so I can argue that being consistent is more important than being
> progressive.
>
> Your call, but since you build next patch on this one, I'm inclined to say
> it's okay to merge it.
It's a NACK from me. That was deliberate. Especially virHashFree doesn't
clear the pointer, the code which we have does.
Oh, and one more reason. Those hash tables are allocated directly by the
glib function since they don't use strings as keys which is the only
wrapper that virHash provides, thus they should not be freed by the
wrappers.
10:25 p.m.
New subject: [PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
On 2/1/21 4:51 AM, Peter Krempa wrote:
On Mon, Feb 01, 2021 at 10:47:47 +0100, Peter Krempa wrote:
> On Mon, Feb 01, 2021 at 10:18:52 +0100, Michal Privoznik wrote:
>> On 2/1/21 7:27 AM, Laine Stump wrote:
>>> virHashFree() just calls g_hash_table_unref(), and it's more common
>>> for libvirt code to call virHashFree() rather than the convoluted
>>> calling of g_hash_table_unref() via g_clear_pointer().
>>>
>>> Since the object containing the hashes is g_freed immediately after
>>> the hashes are freed, there is no functional difference.
>>>
>>> Signed-off-by: Laine Stump <laine(a)redhat.com>
>>> ---
>>> src/conf/domain_addr.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
>>> index 37dad20ade..a8648d5858 100644
>>> --- a/src/conf/domain_addr.c
>>> +++ b/src/conf/domain_addr.c
>>> @@ -949,8 +949,8 @@
virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
>>> if (!addrs || !addrs->zpciIds)
>>> return;
>>> - g_clear_pointer(&addrs->zpciIds->uids, g_hash_table_unref);
>>> - g_clear_pointer(&addrs->zpciIds->fids, g_hash_table_unref);
>>> + virHashFree(addrs->zpciIds->uids);
>>> + virHashFree(addrs->zpciIds->fids);
>>> VIR_FREE(addrs->zpciIds);
>>> }
>>>
>> virHashFree documents itself as being deprecated in favor of
>> g_hash_table_unref().
>>
>> While I like our virSomething wrappers (mostly because I'm used to them more
>> than to their glib counterparts; but then you also have glib functions when
>> one thinks that glib implementation is interchangeable with ours but it
>> isn't - devil's in the details), I think our intent is to drop
>> virHashFree().
>>
>> But then again - we didn't, instead we replaced virHash* internals with
>> glib, so I can argue that being consistent is more important than being
>> progressive.
>>
>> Your call, but since you build next patch on this one, I'm inclined to say
>> it's okay to merge it.
> It's a NACK from me. That was deliberate. Especially virHashFree doesn't
> clear the pointer, the code which we have does.
Oh, and one more reason. Those hash tables are allocated directly by the
glib function since they don't use strings as keys which is the only
wrapper that virHash provides, thus they should not be freed by the
wrappers.
Yeah, I was looking too narrow and didn't notice that. (once again
proving the utility of peer reviews!)
I do still think that we shouldn't be unnecessarily using
g_clear_pointer(), but I'll just drop the patch for now, and maybe
revisit the next time I pass this way.
(NB: If we really think that virHash should be deprecated, then we
should start converting. And if we instead think that its new
g_hash_table-based implementation is a useful value-add on top of the
straight glib API, then we should remove the "deprecated" notes from
virhash.c, and change this usage to (fully, not half-assed like I did)
use virHash like everyone else. No, I'm not volunteering!)
12:27 a.m.
New subject: [PATCH 4/8] conf: fix arg to virDomainPCIAddressSetExtensionFree()
This function clears out and frees a virDomainZPCIAddressIds object,
so that's that's what it should take as its argument, *not* the
pointer to a parent object that contains the object we want to free.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/domain_addr.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index a8648d5858..4ba620086d 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -944,15 +944,15 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs,
static void
-virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs)
+virDomainPCIAddressSetExtensionFree(virDomainZPCIAddressIdsPtr zpciIds)
{
- if (!addrs || !addrs->zpciIds)
+ if (!zpciIds)
return;
- virHashFree(addrs->zpciIds->uids);
- virHashFree(addrs->zpciIds->fids);
+ virHashFree(zpciIds->uids);
+ virHashFree(zpciIds->fids);
- VIR_FREE(addrs->zpciIds);
+ VIR_FREE(zpciIds);
}
@@ -1001,7 +1001,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs)
if (!addrs)
return;
- virDomainPCIAddressSetExtensionFree(addrs);
+ virDomainPCIAddressSetExtensionFree(addrs->zpciIds);
VIR_FREE(addrs->buses);
VIR_FREE(addrs);
}
--
2.29.2
12:27 a.m.
New subject: [PATCH 5/8] conf: don't bother setting pointers to NULL in vir*Free() functions
The memory containing the pointer is going to be freed momentarily anyway.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/domain_conf.c | 5 -----
src/conf/node_device_conf.c | 1 -
2 files changed, 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c16036f5d8..658f563b36 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1908,7 +1908,6 @@ virDomainVcpuDefFree(virDomainVcpuDefPtr info)
return;
virBitmapFree(info->cpumask);
- info->cpumask = NULL;
virObjectUnref(info->privateData);
VIR_FREE(info);
}
@@ -2474,7 +2473,6 @@ virDomainNetDefFree(virDomainNetDefPtr def)
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
virObjectUnref(def->data.vhostuser);
- def->data.vhostuser = NULL;
break;
case VIR_DOMAIN_NET_TYPE_VDPA:
@@ -2493,7 +2491,6 @@ virDomainNetDefFree(virDomainNetDefPtr def)
g_free(def->data.network.name);
g_free(def->data.network.portgroup);
virDomainActualNetDefFree(def->data.network.actual);
- def->data.network.actual = NULL;
break;
case VIR_DOMAIN_NET_TYPE_BRIDGE:
@@ -2537,10 +2534,8 @@ virDomainNetDefFree(virDomainNetDefPtr def)
g_free(def->filter);
virHashFree(def->filterparams);
- def->filterparams = NULL;
virNetDevBandwidthFree(def->bandwidth);
- def->bandwidth = NULL;
virNetDevVlanClear(&def->vlan);
virObjectUnref(def->privateData);
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 35f34b10e9..b6f73161b5 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2422,7 +2422,6 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
VIR_FREE(data->net.ifname);
VIR_FREE(data->net.address);
virBitmapFree(data->net.features);
- data->net.features = NULL;
break;
case VIR_NODE_DEV_CAP_SCSI_HOST:
VIR_FREE(data->scsi_host.wwnn);
--
2.29.2
12:27 a.m.
New subject: [PATCH 6/8] conf: eliminate pointless setting of interface model
There is no point in setting the interface model to unknown during
virDomainNetDefFree(), since we are about to free the object anyway
(and the model isn't used anywhere in the rest of the function).
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/domain_conf.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 658f563b36..1a163857ca 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2468,7 +2468,6 @@ virDomainNetDefFree(virDomainNetDefPtr def)
return;
VIR_FREE(def->modelstr);
- def->model = VIR_DOMAIN_NET_MODEL_UNKNOWN;
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
--
2.29.2
12:27 a.m.
New subject: [PATCH 7/8] util: rename virStorageEncryptionInfoDefFree()
usually a function call vir*Free() will take a single pointer to an
object as its argument, and will then free all resources associated
with that object, including the object
itself. virStorageEnctyptionInfoDefFree() doesn't do that - it frees
all the subordinate resources of the ojbect, but doesn't free the
object itself; usually a function like that is called
vir*Clear(). Let's rename this function to not be misleading.
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
src/conf/storage_encryption_conf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c
index 34ad5dffeb..6a32df15b7 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -48,7 +48,7 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
);
static void
-virStorageEncryptionInfoDefFree(virStorageEncryptionInfoDefPtr def)
+virStorageEncryptionInfoDefClear(virStorageEncryptionInfoDefPtr def)
{
VIR_FREE(def->cipher_name);
VIR_FREE(def->cipher_mode);
@@ -77,7 +77,7 @@ virStorageEncryptionFree(virStorageEncryptionPtr enc)
for (i = 0; i < enc->nsecrets; i++)
virStorageEncryptionSecretFree(enc->secrets[i]);
- virStorageEncryptionInfoDefFree(&enc->encinfo);
+ virStorageEncryptionInfoDefClear(&enc->encinfo);
VIR_FREE(enc->secrets);
VIR_FREE(enc);
}
--
2.29.2
12:27 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
This patch takes on one set of examples of unnecessary use of
VIR_FREE() when g_free() is adequate - it modifies only vir*Free()
functions within the conf directory that take a single pointer and
free the object pointed to by that argument before returning. The
modification is to replace VIR_FREE() with g_free() for the object
itself *and* for all subordinate chunks of memory pointed to by that
object.
(NB: there are other functions that VIR_FREE subordinate memory of
objects that end up being freed before return (also sometimes with
VIR_FREE); I am purposefully ignoring those to reduce scope and focus
on a sub class where the pointlessness is obvious.)
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
NB: After going through the experience a few days ago of needing to
more or less manually backport a patch due to a different patch
similar to this one touching the one function relevant to the desired
patch as well as many dozens of other functions (thus making it
impractical to backport that patch as a prerequisite), I contemplated
splitting this patch up all the way to 1 patch for every
function. That seemed extreme though, so I've left it as is. An
alternative, of course, is to just do nothing and leave everything as
VIR_FREE() (and I know there is sentiment in that direction, a bit
from me even :-P). I wonder though if we'll ever live up to the goals
listed in docs/glib-adoption.txt...
src/conf/capabilities.c | 8 +-
src/conf/cpu_conf.c | 8 +-
src/conf/device_conf.c | 2 +-
src/conf/domain_addr.c | 22 +-
src/conf/domain_capabilities.c | 6 +-
src/conf/domain_conf.c | 298 +++++++++++++-------------
src/conf/interface_conf.c | 32 +--
src/conf/network_conf.c | 18 +-
src/conf/node_device_conf.c | 104 ++++-----
src/conf/nwfilter_conf.c | 22 +-
src/conf/nwfilter_params.c | 16 +-
src/conf/object_event.c | 12 +-
src/conf/secret_conf.c | 6 +-
src/conf/snapshot_conf.c | 2 +-
src/conf/storage_conf.c | 22 +-
src/conf/storage_encryption_conf.c | 4 +-
src/conf/storage_source_conf.c | 26 +--
src/conf/virdomaincheckpointobjlist.c | 2 +-
src/conf/virdomainmomentobjlist.c | 4 +-
src/conf/virdomainsnapshotobjlist.c | 2 +-
src/conf/virnetworkportdef.c | 12 +-
src/conf/virnwfilterbindingdef.c | 10 +-
src/conf/virnwfilterobj.c | 6 +-
23 files changed, 322 insertions(+), 322 deletions(-)
diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 425f34113a..954cdd2c8c 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -207,7 +207,7 @@ virCapsHostMemBWNodeFree(virCapsHostMemBWNodePtr ptr)
return;
virBitmapFree(ptr->cpus);
- VIR_FREE(ptr);
+ g_free(ptr);
}
static void
@@ -1757,9 +1757,9 @@ virCapsHostCacheBankFree(virCapsHostCacheBankPtr ptr)
virBitmapFree(ptr->cpus);
for (i = 0; i < ptr->ncontrols; i++)
- VIR_FREE(ptr->controls[i]);
- VIR_FREE(ptr->controls);
- VIR_FREE(ptr);
+ g_free(ptr->controls[i]);
+ g_free(ptr->controls);
+ g_free(ptr);
}
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index a2d88ba818..f98b0a0eb3 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -127,9 +127,9 @@ virCPUDefFree(virCPUDefPtr def)
if (g_atomic_int_dec_and_test(&def->refs)) {
virCPUDefFreeModel(def);
- VIR_FREE(def->cache);
- VIR_FREE(def->tsc);
- VIR_FREE(def);
+ g_free(def->cache);
+ g_free(def->tsc);
+ g_free(def);
}
}
@@ -1199,5 +1199,5 @@ virCPUDefListFree(virCPUDefPtr *cpus)
for (cpu = cpus; *cpu != NULL; cpu++)
virCPUDefFree(*cpu);
- VIR_FREE(cpus);
+ g_free(cpus);
}
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 87bf32bbc6..714ac50762 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -99,7 +99,7 @@ virDomainDeviceInfoFree(virDomainDeviceInfoPtr info)
{
if (info) {
virDomainDeviceInfoClear(info);
- VIR_FREE(info);
+ g_free(info);
}
}
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 4ba620086d..bbfee6918e 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -952,7 +952,7 @@ virDomainPCIAddressSetExtensionFree(virDomainZPCIAddressIdsPtr
zpciIds)
virHashFree(zpciIds->uids);
virHashFree(zpciIds->fids);
- VIR_FREE(zpciIds);
+ g_free(zpciIds);
}
@@ -1002,8 +1002,8 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs)
return;
virDomainPCIAddressSetExtensionFree(addrs->zpciIds);
- VIR_FREE(addrs->buses);
- VIR_FREE(addrs);
+ g_free(addrs->buses);
+ g_free(addrs);
}
@@ -1379,7 +1379,7 @@ void virDomainCCWAddressSetFree(virDomainCCWAddressSetPtr addrs)
return;
virHashFree(addrs->defined);
- VIR_FREE(addrs);
+ g_free(addrs);
}
static virDomainCCWAddressSetPtr
@@ -1451,7 +1451,7 @@
virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr cont)
{
if (cont) {
virBitmapFree(cont->ports);
- VIR_FREE(cont);
+ g_free(cont);
}
}
@@ -1558,8 +1558,8 @@ virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr
addrs)
if (addrs) {
for (i = 0; i < addrs->ncontrollers; i++)
virDomainVirtioSerialControllerFree(addrs->controllers[i]);
- VIR_FREE(addrs->controllers);
- VIR_FREE(addrs);
+ g_free(addrs->controllers);
+ g_free(addrs);
}
}
@@ -1886,9 +1886,9 @@ virDomainUSBAddressHubFree(virDomainUSBAddressHubPtr hub)
for (i = 0; i < hub->nports; i++)
virDomainUSBAddressHubFree(hub->ports[i]);
- VIR_FREE(hub->ports);
+ g_free(hub->ports);
virBitmapFree(hub->portmap);
- VIR_FREE(hub);
+ g_free(hub);
}
@@ -1902,8 +1902,8 @@ virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs)
for (i = 0; i < addrs->nbuses; i++)
virDomainUSBAddressHubFree(addrs->buses[i]);
- VIR_FREE(addrs->buses);
- VIR_FREE(addrs);
+ g_free(addrs->buses);
+ g_free(addrs);
}
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 8130311590..08789aeb31 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -85,9 +85,9 @@ virSEVCapabilitiesFree(virSEVCapability *cap)
if (!cap)
return;
- VIR_FREE(cap->pdh);
- VIR_FREE(cap->cert_chain);
- VIR_FREE(cap);
+ g_free(cap->pdh);
+ g_free(cap->cert_chain);
+ g_free(cap);
}
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1a163857ca..97fa841bff 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1806,30 +1806,30 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
switch (def->type) {
case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
- VIR_FREE(def->data.vnc.keymap);
+ g_free(def->data.vnc.keymap);
virDomainGraphicsAuthDefClear(&def->data.vnc.auth);
break;
case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
- VIR_FREE(def->data.sdl.display);
- VIR_FREE(def->data.sdl.xauth);
+ g_free(def->data.sdl.display);
+ g_free(def->data.sdl.xauth);
break;
case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
break;
case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
- VIR_FREE(def->data.desktop.display);
+ g_free(def->data.desktop.display);
break;
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
- VIR_FREE(def->data.spice.rendernode);
- VIR_FREE(def->data.spice.keymap);
+ g_free(def->data.spice.rendernode);
+ g_free(def->data.spice.keymap);
virDomainGraphicsAuthDefClear(&def->data.spice.auth);
break;
case VIR_DOMAIN_GRAPHICS_TYPE_EGL_HEADLESS:
- VIR_FREE(def->data.egl_headless.rendernode);
+ g_free(def->data.egl_headless.rendernode);
break;
case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
@@ -1838,10 +1838,10 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def)
for (i = 0; i < def->nListens; i++)
virDomainGraphicsListenDefClear(&def->listens[i]);
- VIR_FREE(def->listens);
+ g_free(def->listens);
virObjectUnref(def->privateData);
- VIR_FREE(def);
+ g_free(def);
}
const char *virDomainInputDefGetPath(virDomainInputDefPtr input)
@@ -1865,9 +1865,9 @@ void virDomainInputDefFree(virDomainInputDefPtr def)
return;
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def->source.evdev);
- VIR_FREE(def->virtio);
- VIR_FREE(def);
+ g_free(def->source.evdev);
+ g_free(def->virtio);
+ g_free(def);
}
void virDomainLeaseDefFree(virDomainLeaseDefPtr def)
@@ -1875,11 +1875,11 @@ void virDomainLeaseDefFree(virDomainLeaseDefPtr def)
if (!def)
return;
- VIR_FREE(def->lockspace);
- VIR_FREE(def->key);
- VIR_FREE(def->path);
+ g_free(def->lockspace);
+ g_free(def->key);
+ g_free(def->path);
- VIR_FREE(def);
+ g_free(def);
}
@@ -1909,7 +1909,7 @@ virDomainVcpuDefFree(virDomainVcpuDefPtr info)
virBitmapFree(info->cpumask);
virObjectUnref(info->privateData);
- VIR_FREE(info);
+ g_free(info);
}
@@ -2184,20 +2184,20 @@ virDomainDiskDefFree(virDomainDiskDefPtr def)
return;
virObjectUnref(def->src);
- VIR_FREE(def->serial);
- VIR_FREE(def->dst);
+ g_free(def->serial);
+ g_free(def->dst);
virObjectUnref(def->mirror);
- VIR_FREE(def->wwn);
- VIR_FREE(def->driverName);
- VIR_FREE(def->vendor);
- VIR_FREE(def->product);
- VIR_FREE(def->domain_name);
- VIR_FREE(def->blkdeviotune.group_name);
- VIR_FREE(def->virtio);
+ g_free(def->wwn);
+ g_free(def->driverName);
+ g_free(def->vendor);
+ g_free(def->product);
+ g_free(def->domain_name);
+ g_free(def->blkdeviotune.group_name);
+ g_free(def->virtio);
virDomainDeviceInfoClear(&def->info);
virObjectUnref(def->privateData);
- VIR_FREE(def);
+ g_free(def);
}
@@ -2327,9 +2327,9 @@ void virDomainControllerDefFree(virDomainControllerDefPtr def)
return;
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def->virtio);
+ g_free(def->virtio);
- VIR_FREE(def);
+ g_free(def);
}
@@ -2391,13 +2391,13 @@ void virDomainFSDefFree(virDomainFSDefPtr def)
return;
virObjectUnref(def->src);
- VIR_FREE(def->dst);
+ g_free(def->dst);
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def->virtio);
+ g_free(def->virtio);
virObjectUnref(def->privateData);
- VIR_FREE(def->binary);
+ g_free(def->binary);
- VIR_FREE(def);
+ g_free(def);
}
void
@@ -2409,10 +2409,10 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
case VIR_DOMAIN_NET_TYPE_NETWORK:
- VIR_FREE(def->data.bridge.brname);
+ g_free(def->data.bridge.brname);
break;
case VIR_DOMAIN_NET_TYPE_DIRECT:
- VIR_FREE(def->data.direct.linkdev);
+ g_free(def->data.direct.linkdev);
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
virDomainHostdevDefClear(&def->data.hostdev.def);
@@ -2421,10 +2421,10 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
break;
}
- VIR_FREE(def->virtPortProfile);
+ g_free(def->virtPortProfile);
virNetDevBandwidthFree(def->bandwidth);
virNetDevVlanClear(&def->vlan);
- VIR_FREE(def);
+ g_free(def);
}
@@ -2456,8 +2456,8 @@ virDomainVsockDefFree(virDomainVsockDefPtr vsock)
virObjectUnref(vsock->privateData);
virDomainDeviceInfoClear(&vsock->info);
- VIR_FREE(vsock->virtio);
- VIR_FREE(vsock);
+ g_free(vsock->virtio);
+ g_free(vsock);
}
@@ -2467,7 +2467,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
if (!def)
return;
- VIR_FREE(def->modelstr);
+ g_free(def->modelstr);
switch (def->type) {
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
@@ -2475,7 +2475,7 @@ virDomainNetDefFree(virDomainNetDefPtr def)
break;
case VIR_DOMAIN_NET_TYPE_VDPA:
- VIR_FREE(def->data.vdpa.devicepath);
+ g_free(def->data.vdpa.devicepath);
break;
case VIR_DOMAIN_NET_TYPE_SERVER:
@@ -2764,12 +2764,12 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
switch (def->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
- VIR_FREE(def->target.addr);
+ g_free(def->target.addr);
break;
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN:
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO:
- VIR_FREE(def->target.name);
+ g_free(def->target.name);
break;
}
break;
@@ -2781,7 +2781,7 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
virObjectUnref(def->source);
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def)
@@ -2796,8 +2796,8 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def)
case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++)
- VIR_FREE(def->data.cert.file[i]);
- VIR_FREE(def->data.cert.database);
+ g_free(def->data.cert.file[i]);
+ g_free(def->data.cert.database);
break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
@@ -2810,7 +2810,7 @@ void virDomainSmartcardDefFree(virDomainSmartcardDefPtr def)
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def)
@@ -2818,7 +2818,7 @@ void virDomainSoundCodecDefFree(virDomainSoundCodecDefPtr def)
if (!def)
return;
- VIR_FREE(def);
+ g_free(def);
}
void virDomainSoundDefFree(virDomainSoundDefPtr def)
@@ -2832,9 +2832,9 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def)
for (i = 0; i < def->ncodecs; i++)
virDomainSoundCodecDefFree(def->codecs[i]);
- VIR_FREE(def->codecs);
+ g_free(def->codecs);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainAudioDefFree(virDomainAudioDefPtr def)
@@ -2844,15 +2844,15 @@ void virDomainAudioDefFree(virDomainAudioDefPtr def)
switch ((virDomainAudioType) def->type) {
case VIR_DOMAIN_AUDIO_TYPE_OSS:
- VIR_FREE(def->backend.oss.inputDev);
- VIR_FREE(def->backend.oss.outputDev);
+ g_free(def->backend.oss.inputDev);
+ g_free(def->backend.oss.outputDev);
break;
case VIR_DOMAIN_AUDIO_TYPE_LAST:
break;
}
- VIR_FREE(def);
+ g_free(def);
}
virDomainSoundDefPtr
@@ -2869,9 +2869,9 @@ void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def)
return;
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def->virtio);
+ g_free(def->virtio);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def)
@@ -2881,7 +2881,7 @@ void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def)
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def)
@@ -2891,7 +2891,7 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def)
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainShmemDefFree(virDomainShmemDefPtr def)
@@ -2901,8 +2901,8 @@ void virDomainShmemDefFree(virDomainShmemDefPtr def)
virDomainDeviceInfoClear(&def->info);
virDomainChrSourceDefClear(&def->server.chr);
- VIR_FREE(def->name);
- VIR_FREE(def);
+ g_free(def->name);
+ g_free(def);
}
@@ -2952,7 +2952,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
return;
virDomainVideoDefClear(def);
- VIR_FREE(def);
+ g_free(def);
}
@@ -3044,15 +3044,15 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def)
break;
case VIR_DOMAIN_TPM_TYPE_EMULATOR:
virDomainChrSourceDefClear(&def->data.emulator.source);
- VIR_FREE(def->data.emulator.storagepath);
- VIR_FREE(def->data.emulator.logfile);
+ g_free(def->data.emulator.storagepath);
+ g_free(def->data.emulator.logfile);
break;
case VIR_DOMAIN_TPM_TYPE_LAST:
break;
}
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
@@ -3067,7 +3067,7 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
* the memory.
*/
if (!def->parentnet)
- VIR_FREE(def);
+ g_free(def);
}
void virDomainHubDefFree(virDomainHubDefPtr def)
@@ -3076,7 +3076,7 @@ void virDomainHubDefFree(virDomainHubDefPtr def)
return;
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def)
@@ -3087,7 +3087,7 @@ void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def)
virObjectUnref(def->source);
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def)
@@ -3098,10 +3098,10 @@ void virDomainRedirFilterDefFree(virDomainRedirFilterDefPtr def)
return;
for (i = 0; i < def->nusbdevs; i++)
- VIR_FREE(def->usbdevs[i]);
+ g_free(def->usbdevs[i]);
- VIR_FREE(def->usbdevs);
- VIR_FREE(def);
+ g_free(def->usbdevs);
+ g_free(def);
}
void virDomainMemoryDefFree(virDomainMemoryDefPtr def)
@@ -3109,11 +3109,11 @@ void virDomainMemoryDefFree(virDomainMemoryDefPtr def)
if (!def)
return;
- VIR_FREE(def->nvdimmPath);
+ g_free(def->nvdimmPath);
virBitmapFree(def->sourceNodes);
- VIR_FREE(def->uuid);
+ g_free(def->uuid);
virDomainDeviceInfoClear(&def->info);
- VIR_FREE(def);
+ g_free(def);
}
void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
@@ -3189,7 +3189,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
virDomainMemoryDefFree(def->data.memory);
break;
case VIR_DOMAIN_DEVICE_IOMMU:
- VIR_FREE(def->data.iommu);
+ g_free(def->data.iommu);
break;
case VIR_DOMAIN_DEVICE_VSOCK:
virDomainVsockDefFree(def->data.vsock);
@@ -3202,7 +3202,7 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
break;
}
- VIR_FREE(def);
+ g_free(def);
}
static void
@@ -3238,7 +3238,7 @@ virDomainIOThreadIDDefFree(virDomainIOThreadIDDefPtr def)
if (!def)
return;
virBitmapFree(def->cpumask);
- VIR_FREE(def);
+ g_free(def);
}
@@ -3254,7 +3254,7 @@ virDomainIOThreadIDDefArrayFree(virDomainIOThreadIDDefPtr *def,
for (i = 0; i < nids; i++)
virDomainIOThreadIDDefFree(def[i]);
- VIR_FREE(def);
+ g_free(def);
}
@@ -3311,8 +3311,8 @@ virDomainResourceDefFree(virDomainResourceDefPtr resource)
if (!resource)
return;
- VIR_FREE(resource->partition);
- VIR_FREE(resource);
+ g_free(resource->partition);
+ g_free(resource);
}
void
@@ -3322,7 +3322,7 @@ virDomainPanicDefFree(virDomainPanicDefPtr panic)
return;
virDomainDeviceInfoClear(&panic->info);
- VIR_FREE(panic);
+ g_free(panic);
}
void
@@ -3331,10 +3331,10 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader)
if (!loader)
return;
- VIR_FREE(loader->path);
- VIR_FREE(loader->nvram);
- VIR_FREE(loader->templt);
- VIR_FREE(loader);
+ g_free(loader->path);
+ g_free(loader->nvram);
+ g_free(loader->templt);
+ g_free(loader);
}
@@ -3346,7 +3346,7 @@ virDomainResctrlMonDefFree(virDomainResctrlMonDefPtr domresmon)
virBitmapFree(domresmon->vcpus);
virObjectUnref(domresmon->instance);
- VIR_FREE(domresmon);
+ g_free(domresmon);
}
@@ -3363,8 +3363,8 @@ virDomainResctrlDefFree(virDomainResctrlDefPtr resctrl)
virObjectUnref(resctrl->alloc);
virBitmapFree(resctrl->vcpus);
- VIR_FREE(resctrl->monitors);
- VIR_FREE(resctrl);
+ g_free(resctrl->monitors);
+ g_free(resctrl);
}
@@ -3374,10 +3374,10 @@ virDomainSEVDefFree(virDomainSEVDefPtr def)
if (!def)
return;
- VIR_FREE(def->dh_cert);
- VIR_FREE(def->session);
+ g_free(def->dh_cert);
+ g_free(def->session);
- VIR_FREE(def);
+ g_free(def);
}
@@ -3392,7 +3392,7 @@ void virDomainDefFree(virDomainDefPtr def)
for (i = 0; i < def->maxvcpus; i++)
virDomainVcpuDefFree(def->vcpus[i]);
- VIR_FREE(def->vcpus);
+ g_free(def->vcpus);
/* hostdevs must be freed before nets (or any future "intelligent
* hostdevs") because the pointer to the hostdev is really
@@ -3402,133 +3402,133 @@ void virDomainDefFree(virDomainDefPtr def)
*/
for (i = 0; i < def->nhostdevs; i++)
virDomainHostdevDefFree(def->hostdevs[i]);
- VIR_FREE(def->hostdevs);
+ g_free(def->hostdevs);
for (i = 0; i < def->nleases; i++)
virDomainLeaseDefFree(def->leases[i]);
- VIR_FREE(def->leases);
+ g_free(def->leases);
for (i = 0; i < def->ngraphics; i++)
virDomainGraphicsDefFree(def->graphics[i]);
- VIR_FREE(def->graphics);
+ g_free(def->graphics);
for (i = 0; i < def->ninputs; i++)
virDomainInputDefFree(def->inputs[i]);
- VIR_FREE(def->inputs);
+ g_free(def->inputs);
for (i = 0; i < def->ndisks; i++)
virDomainDiskDefFree(def->disks[i]);
- VIR_FREE(def->disks);
+ g_free(def->disks);
for (i = 0; i < def->ncontrollers; i++)
virDomainControllerDefFree(def->controllers[i]);
- VIR_FREE(def->controllers);
+ g_free(def->controllers);
for (i = 0; i < def->nfss; i++)
virDomainFSDefFree(def->fss[i]);
- VIR_FREE(def->fss);
+ g_free(def->fss);
for (i = 0; i < def->nnets; i++)
virDomainNetDefFree(def->nets[i]);
- VIR_FREE(def->nets);
+ g_free(def->nets);
for (i = 0; i < def->nsmartcards; i++)
virDomainSmartcardDefFree(def->smartcards[i]);
- VIR_FREE(def->smartcards);
+ g_free(def->smartcards);
for (i = 0; i < def->nserials; i++)
virDomainChrDefFree(def->serials[i]);
- VIR_FREE(def->serials);
+ g_free(def->serials);
for (i = 0; i < def->nparallels; i++)
virDomainChrDefFree(def->parallels[i]);
- VIR_FREE(def->parallels);
+ g_free(def->parallels);
for (i = 0; i < def->nchannels; i++)
virDomainChrDefFree(def->channels[i]);
- VIR_FREE(def->channels);
+ g_free(def->channels);
for (i = 0; i < def->nconsoles; i++)
virDomainChrDefFree(def->consoles[i]);
- VIR_FREE(def->consoles);
+ g_free(def->consoles);
for (i = 0; i < def->nsounds; i++)
virDomainSoundDefFree(def->sounds[i]);
- VIR_FREE(def->sounds);
+ g_free(def->sounds);
for (i = 0; i < def->naudios; i++)
virDomainAudioDefFree(def->audios[i]);
- VIR_FREE(def->audios);
+ g_free(def->audios);
for (i = 0; i < def->nvideos; i++)
virDomainVideoDefFree(def->videos[i]);
- VIR_FREE(def->videos);
+ g_free(def->videos);
for (i = 0; i < def->nhubs; i++)
virDomainHubDefFree(def->hubs[i]);
- VIR_FREE(def->hubs);
+ g_free(def->hubs);
for (i = 0; i < def->nredirdevs; i++)
virDomainRedirdevDefFree(def->redirdevs[i]);
- VIR_FREE(def->redirdevs);
+ g_free(def->redirdevs);
for (i = 0; i < def->nrngs; i++)
virDomainRNGDefFree(def->rngs[i]);
- VIR_FREE(def->rngs);
+ g_free(def->rngs);
for (i = 0; i < def->nmems; i++)
virDomainMemoryDefFree(def->mems[i]);
- VIR_FREE(def->mems);
+ g_free(def->mems);
for (i = 0; i < def->ntpms; i++)
virDomainTPMDefFree(def->tpms[i]);
- VIR_FREE(def->tpms);
+ g_free(def->tpms);
for (i = 0; i < def->npanics; i++)
virDomainPanicDefFree(def->panics[i]);
- VIR_FREE(def->panics);
+ g_free(def->panics);
- VIR_FREE(def->iommu);
+ g_free(def->iommu);
- VIR_FREE(def->idmap.uidmap);
- VIR_FREE(def->idmap.gidmap);
+ g_free(def->idmap.uidmap);
+ g_free(def->idmap.gidmap);
- VIR_FREE(def->os.machine);
- VIR_FREE(def->os.init);
+ g_free(def->os.machine);
+ g_free(def->os.init);
for (i = 0; def->os.initargv && def->os.initargv[i]; i++)
- VIR_FREE(def->os.initargv[i]);
- VIR_FREE(def->os.initargv);
+ g_free(def->os.initargv[i]);
+ g_free(def->os.initargv);
for (i = 0; def->os.initenv && def->os.initenv[i]; i++) {
- VIR_FREE(def->os.initenv[i]->name);
- VIR_FREE(def->os.initenv[i]->value);
- VIR_FREE(def->os.initenv[i]);
- }
- VIR_FREE(def->os.initdir);
- VIR_FREE(def->os.inituser);
- VIR_FREE(def->os.initgroup);
- VIR_FREE(def->os.initenv);
- VIR_FREE(def->os.kernel);
- VIR_FREE(def->os.initrd);
- VIR_FREE(def->os.cmdline);
- VIR_FREE(def->os.dtb);
- VIR_FREE(def->os.root);
- VIR_FREE(def->os.slic_table);
+ g_free(def->os.initenv[i]->name);
+ g_free(def->os.initenv[i]->value);
+ g_free(def->os.initenv[i]);
+ }
+ g_free(def->os.initdir);
+ g_free(def->os.inituser);
+ g_free(def->os.initgroup);
+ g_free(def->os.initenv);
+ g_free(def->os.kernel);
+ g_free(def->os.initrd);
+ g_free(def->os.cmdline);
+ g_free(def->os.dtb);
+ g_free(def->os.root);
+ g_free(def->os.slic_table);
virDomainLoaderDefFree(def->os.loader);
- VIR_FREE(def->os.bootloader);
- VIR_FREE(def->os.bootloaderArgs);
+ g_free(def->os.bootloader);
+ g_free(def->os.bootloaderArgs);
virDomainClockDefClear(&def->clock);
- VIR_FREE(def->name);
+ g_free(def->name);
virBitmapFree(def->cpumask);
- VIR_FREE(def->emulator);
- VIR_FREE(def->description);
- VIR_FREE(def->title);
- VIR_FREE(def->hyperv_vendor_id);
+ g_free(def->emulator);
+ g_free(def->description);
+ g_free(def->title);
+ g_free(def->hyperv_vendor_id);
virBlkioDeviceArrayClear(def->blkio.devices,
def->blkio.ndevices);
- VIR_FREE(def->blkio.devices);
+ g_free(def->blkio.devices);
virDomainWatchdogDefFree(def->watchdog);
@@ -3538,36 +3538,36 @@ void virDomainDefFree(virDomainDefPtr def)
for (i = 0; i < def->mem.nhugepages; i++)
virBitmapFree(def->mem.hugepages[i].nodemask);
- VIR_FREE(def->mem.hugepages);
+ g_free(def->mem.hugepages);
for (i = 0; i < def->nseclabels; i++)
virSecurityLabelDefFree(def->seclabels[i]);
- VIR_FREE(def->seclabels);
+ g_free(def->seclabels);
virCPUDefFree(def->cpu);
virDomainIOThreadIDDefArrayFree(def->iothreadids, def->niothreadids);
virBitmapFree(def->cputune.emulatorpin);
- VIR_FREE(def->cputune.emulatorsched);
+ g_free(def->cputune.emulatorsched);
virDomainNumaFree(def->numa);
for (i = 0; i < def->nsysinfo; i++)
virSysinfoDefFree(def->sysinfo[i]);
- VIR_FREE(def->sysinfo);
+ g_free(def->sysinfo);
virDomainRedirFilterDefFree(def->redirfilter);
for (i = 0; i < def->nshmems; i++)
virDomainShmemDefFree(def->shmems[i]);
- VIR_FREE(def->shmems);
+ g_free(def->shmems);
for (i = 0; i < def->nresctrls; i++)
virDomainResctrlDefFree(def->resctrls[i]);
- VIR_FREE(def->resctrls);
+ g_free(def->resctrls);
- VIR_FREE(def->keywrap);
+ g_free(def->keywrap);
if (def->namespaceData && def->ns.free)
(def->ns.free)(def->namespaceData);
@@ -3576,7 +3576,7 @@ void virDomainDefFree(virDomainDefPtr def)
xmlFreeNode(def->metadata);
- VIR_FREE(def);
+ g_free(def);
}
static void virDomainObjDispose(void *obj)
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 7bb5ec4deb..3cd8fcee94 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -49,8 +49,8 @@ virInterfaceIPDefFree(virInterfaceIPDefPtr def)
{
if (def == NULL)
return;
- VIR_FREE(def->address);
- VIR_FREE(def);
+ g_free(def->address);
+ g_free(def);
}
@@ -63,10 +63,10 @@ virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def)
return;
for (i = 0; i < def->nips; i++)
virInterfaceIPDefFree(def->ips[i]);
- VIR_FREE(def->ips);
- VIR_FREE(def->family);
- VIR_FREE(def->gateway);
- VIR_FREE(def);
+ g_free(def->ips);
+ g_free(def->family);
+ g_free(def->gateway);
+ g_free(def);
}
@@ -79,39 +79,39 @@ virInterfaceDefFree(virInterfaceDefPtr def)
if (def == NULL)
return;
- VIR_FREE(def->name);
- VIR_FREE(def->mac);
+ g_free(def->name);
+ g_free(def->mac);
switch (def->type) {
case VIR_INTERFACE_TYPE_BRIDGE:
- VIR_FREE(def->data.bridge.delay);
+ g_free(def->data.bridge.delay);
for (i = 0; i < def->data.bridge.nbItf; i++) {
if (def->data.bridge.itf[i] == NULL)
break; /* to cope with half parsed data on errors */
virInterfaceDefFree(def->data.bridge.itf[i]);
}
- VIR_FREE(def->data.bridge.itf);
+ g_free(def->data.bridge.itf);
break;
case VIR_INTERFACE_TYPE_BOND:
- VIR_FREE(def->data.bond.target);
+ g_free(def->data.bond.target);
for (i = 0; i < def->data.bond.nbItf; i++) {
if (def->data.bond.itf[i] == NULL)
break; /* to cope with half parsed data on errors */
virInterfaceDefFree(def->data.bond.itf[i]);
}
- VIR_FREE(def->data.bond.itf);
+ g_free(def->data.bond.itf);
break;
case VIR_INTERFACE_TYPE_VLAN:
- VIR_FREE(def->data.vlan.tag);
- VIR_FREE(def->data.vlan.dev_name);
+ g_free(def->data.vlan.tag);
+ g_free(def->data.vlan.dev_name);
break;
}
/* free all protos */
for (pp = 0; pp < def->nprotos; pp++)
virInterfaceProtocolDefFree(def->protos[pp]);
- VIR_FREE(def->protos);
- VIR_FREE(def);
+ g_free(def->protos);
+ g_free(def);
}
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ff7a56f4f4..f32710b781 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -258,28 +258,28 @@ virNetworkDefFree(virNetworkDefPtr def)
if (!def)
return;
- VIR_FREE(def->name);
- VIR_FREE(def->bridge);
- VIR_FREE(def->bridgeZone);
- VIR_FREE(def->domain);
+ g_free(def->name);
+ g_free(def->bridge);
+ g_free(def->bridgeZone);
+ g_free(def->domain);
virNetworkForwardDefClear(&def->forward);
for (i = 0; i < def->nips && def->ips; i++)
virNetworkIPDefClear(&def->ips[i]);
- VIR_FREE(def->ips);
+ g_free(def->ips);
for (i = 0; i < def->nroutes && def->routes; i++)
virNetDevIPRouteFree(def->routes[i]);
- VIR_FREE(def->routes);
+ g_free(def->routes);
for (i = 0; i < def->nPortGroups && def->portGroups; i++)
virPortGroupDefClear(&def->portGroups[i]);
- VIR_FREE(def->portGroups);
+ g_free(def->portGroups);
virNetworkDNSDefClear(&def->dns);
- VIR_FREE(def->virtPortProfile);
+ g_free(def->virtPortProfile);
virNetDevBandwidthFree(def->bandwidth);
virNetDevVlanClear(&def->vlan);
@@ -288,7 +288,7 @@ virNetworkDefFree(virNetworkDefPtr def)
if (def->namespaceData && def->ns.free)
(def->ns.free)(def->namespaceData);
- VIR_FREE(def);
+ g_free(def);
}
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index b6f73161b5..1093a461af 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -108,15 +108,15 @@ virNodeDeviceDefFree(virNodeDeviceDefPtr def)
if (!def)
return;
- VIR_FREE(def->name);
- VIR_FREE(def->parent);
- VIR_FREE(def->parent_wwnn);
- VIR_FREE(def->parent_wwpn);
- VIR_FREE(def->parent_fabric_wwn);
- VIR_FREE(def->driver);
- VIR_FREE(def->sysfs_path);
- VIR_FREE(def->parent_sysfs_path);
- VIR_FREE(def->devnode);
+ g_free(def->name);
+ g_free(def->parent);
+ g_free(def->parent_wwnn);
+ g_free(def->parent_wwpn);
+ g_free(def->parent_fabric_wwn);
+ g_free(def->driver);
+ g_free(def->sysfs_path);
+ g_free(def->parent_sysfs_path);
+ g_free(def->devnode);
g_strfreev(def->devlinks);
caps = def->caps;
@@ -126,7 +126,7 @@ virNodeDeviceDefFree(virNodeDeviceDefPtr def)
caps = next;
}
- VIR_FREE(def);
+ g_free(def);
}
@@ -2388,83 +2388,83 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
switch (caps->data.type) {
case VIR_NODE_DEV_CAP_SYSTEM:
- VIR_FREE(data->system.product_name);
- VIR_FREE(data->system.hardware.vendor_name);
- VIR_FREE(data->system.hardware.version);
- VIR_FREE(data->system.hardware.serial);
- VIR_FREE(data->system.firmware.vendor_name);
- VIR_FREE(data->system.firmware.version);
- VIR_FREE(data->system.firmware.release_date);
+ g_free(data->system.product_name);
+ g_free(data->system.hardware.vendor_name);
+ g_free(data->system.hardware.version);
+ g_free(data->system.hardware.serial);
+ g_free(data->system.firmware.vendor_name);
+ g_free(data->system.firmware.version);
+ g_free(data->system.firmware.release_date);
break;
case VIR_NODE_DEV_CAP_PCI_DEV:
- VIR_FREE(data->pci_dev.product_name);
- VIR_FREE(data->pci_dev.vendor_name);
- VIR_FREE(data->pci_dev.physical_function);
+ g_free(data->pci_dev.product_name);
+ g_free(data->pci_dev.vendor_name);
+ g_free(data->pci_dev.physical_function);
for (i = 0; i < data->pci_dev.num_virtual_functions; i++)
- VIR_FREE(data->pci_dev.virtual_functions[i]);
- VIR_FREE(data->pci_dev.virtual_functions);
+ g_free(data->pci_dev.virtual_functions[i]);
+ g_free(data->pci_dev.virtual_functions);
for (i = 0; i < data->pci_dev.nIommuGroupDevices; i++)
- VIR_FREE(data->pci_dev.iommuGroupDevices[i]);
- VIR_FREE(data->pci_dev.iommuGroupDevices);
+ g_free(data->pci_dev.iommuGroupDevices[i]);
+ g_free(data->pci_dev.iommuGroupDevices);
virPCIEDeviceInfoFree(data->pci_dev.pci_express);
for (i = 0; i < data->pci_dev.nmdev_types; i++)
virMediatedDeviceTypeFree(data->pci_dev.mdev_types[i]);
- VIR_FREE(data->pci_dev.mdev_types);
+ g_free(data->pci_dev.mdev_types);
break;
case VIR_NODE_DEV_CAP_USB_DEV:
- VIR_FREE(data->usb_dev.product_name);
- VIR_FREE(data->usb_dev.vendor_name);
+ g_free(data->usb_dev.product_name);
+ g_free(data->usb_dev.vendor_name);
break;
case VIR_NODE_DEV_CAP_USB_INTERFACE:
- VIR_FREE(data->usb_if.description);
+ g_free(data->usb_if.description);
break;
case VIR_NODE_DEV_CAP_NET:
- VIR_FREE(data->net.ifname);
- VIR_FREE(data->net.address);
+ g_free(data->net.ifname);
+ g_free(data->net.address);
virBitmapFree(data->net.features);
break;
case VIR_NODE_DEV_CAP_SCSI_HOST:
- VIR_FREE(data->scsi_host.wwnn);
- VIR_FREE(data->scsi_host.wwpn);
- VIR_FREE(data->scsi_host.fabric_wwn);
+ g_free(data->scsi_host.wwnn);
+ g_free(data->scsi_host.wwpn);
+ g_free(data->scsi_host.fabric_wwn);
break;
case VIR_NODE_DEV_CAP_SCSI_TARGET:
- VIR_FREE(data->scsi_target.name);
- VIR_FREE(data->scsi_target.rport);
- VIR_FREE(data->scsi_target.wwpn);
+ g_free(data->scsi_target.name);
+ g_free(data->scsi_target.rport);
+ g_free(data->scsi_target.wwpn);
break;
case VIR_NODE_DEV_CAP_SCSI:
- VIR_FREE(data->scsi.type);
+ g_free(data->scsi.type);
break;
case VIR_NODE_DEV_CAP_STORAGE:
- VIR_FREE(data->storage.block);
- VIR_FREE(data->storage.bus);
- VIR_FREE(data->storage.drive_type);
- VIR_FREE(data->storage.model);
- VIR_FREE(data->storage.vendor);
- VIR_FREE(data->storage.serial);
- VIR_FREE(data->storage.media_label);
+ g_free(data->storage.block);
+ g_free(data->storage.bus);
+ g_free(data->storage.drive_type);
+ g_free(data->storage.model);
+ g_free(data->storage.vendor);
+ g_free(data->storage.serial);
+ g_free(data->storage.media_label);
break;
case VIR_NODE_DEV_CAP_SCSI_GENERIC:
- VIR_FREE(data->sg.path);
+ g_free(data->sg.path);
break;
case VIR_NODE_DEV_CAP_MDEV:
- VIR_FREE(data->mdev.type);
- VIR_FREE(data->mdev.uuid);
+ g_free(data->mdev.type);
+ g_free(data->mdev.uuid);
for (i = 0; i < data->mdev.nattributes; i++)
virMediatedDeviceAttrFree(data->mdev.attributes[i]);
- VIR_FREE(data->mdev.attributes);
+ g_free(data->mdev.attributes);
break;
case VIR_NODE_DEV_CAP_CSS_DEV:
for (i = 0; i < data->ccw_dev.nmdev_types; i++)
virMediatedDeviceTypeFree(data->ccw_dev.mdev_types[i]);
- VIR_FREE(data->ccw_dev.mdev_types);
+ g_free(data->ccw_dev.mdev_types);
break;
case VIR_NODE_DEV_CAP_AP_MATRIX:
- VIR_FREE(data->ap_matrix.addr);
+ g_free(data->ap_matrix.addr);
for (i = 0; i < data->ap_matrix.nmdev_types; i++)
virMediatedDeviceTypeFree(data->ap_matrix.mdev_types[i]);
- VIR_FREE(data->ap_matrix.mdev_types);
+ g_free(data->ap_matrix.mdev_types);
break;
case VIR_NODE_DEV_CAP_MDEV_TYPES:
case VIR_NODE_DEV_CAP_DRM:
@@ -2479,7 +2479,7 @@ virNodeDevCapsDefFree(virNodeDevCapsDefPtr caps)
break;
}
- VIR_FREE(caps);
+ g_free(caps);
}
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index be684c3512..fbb87b7acf 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -309,12 +309,12 @@ virNWFilterRuleDefFree(virNWFilterRuleDefPtr def)
virNWFilterVarAccessFree(def->varAccess[i]);
for (i = 0; i < def->nstrings; i++)
- VIR_FREE(def->strings[i]);
+ g_free(def->strings[i]);
- VIR_FREE(def->varAccess);
- VIR_FREE(def->strings);
+ g_free(def->varAccess);
+ g_free(def->strings);
- VIR_FREE(def);
+ g_free(def);
}
@@ -324,8 +324,8 @@ virNWFilterIncludeDefFree(virNWFilterIncludeDefPtr inc)
if (!inc)
return;
virHashFree(inc->params);
- VIR_FREE(inc->filterref);
- VIR_FREE(inc);
+ g_free(inc->filterref);
+ g_free(inc);
}
@@ -337,7 +337,7 @@ virNWFilterEntryFree(virNWFilterEntryPtr entry)
virNWFilterRuleDefFree(entry->rule);
virNWFilterIncludeDefFree(entry->include);
- VIR_FREE(entry);
+ g_free(entry);
}
@@ -348,15 +348,15 @@ virNWFilterDefFree(virNWFilterDefPtr def)
if (!def)
return;
- VIR_FREE(def->name);
+ g_free(def->name);
for (i = 0; i < def->nentries; i++)
virNWFilterEntryFree(def->filterEntries[i]);
- VIR_FREE(def->filterEntries);
- VIR_FREE(def->chainsuffix);
+ g_free(def->filterEntries);
+ g_free(def->chainsuffix);
- VIR_FREE(def);
+ g_free(def);
}
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index ae819024ad..18b64e373b 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -50,17 +50,17 @@ virNWFilterVarValueFree(virNWFilterVarValuePtr val)
switch (val->valType) {
case NWFILTER_VALUE_TYPE_SIMPLE:
- VIR_FREE(val->u.simple.value);
+ g_free(val->u.simple.value);
break;
case NWFILTER_VALUE_TYPE_ARRAY:
for (i = 0; i < val->u.array.nValues; i++)
- VIR_FREE(val->u.array.values[i]);
- VIR_FREE(val->u.array.values);
+ g_free(val->u.array.values[i]);
+ g_free(val->u.array.values);
break;
case NWFILTER_VALUE_TYPE_LAST:
break;
}
- VIR_FREE(val);
+ g_free(val);
}
virNWFilterVarValuePtr
@@ -302,9 +302,9 @@ virNWFilterVarCombIterFree(virNWFilterVarCombIterPtr ci)
return;
for (i = 0; i < ci->nIter; i++)
- VIR_FREE(ci->iter[i].varNames);
+ g_free(ci->iter[i].varNames);
- VIR_FREE(ci);
+ g_free(ci);
}
static int
@@ -809,8 +809,8 @@ virNWFilterVarAccessFree(virNWFilterVarAccessPtr varAccess)
if (!varAccess)
return;
- VIR_FREE(varAccess->varName);
- VIR_FREE(varAccess);
+ g_free(varAccess->varName);
+ g_free(varAccess);
}
bool
diff --git a/src/conf/object_event.c b/src/conf/object_event.c
index 28e75f8b59..907aa6d06a 100644
--- a/src/conf/object_event.c
+++ b/src/conf/object_event.c
@@ -140,8 +140,8 @@ virObjectEventCallbackFree(virObjectEventCallbackPtr cb)
return;
virObjectUnref(cb->conn);
- VIR_FREE(cb->key);
- VIR_FREE(cb);
+ g_free(cb->key);
+ g_free(cb);
}
/**
@@ -161,10 +161,10 @@ virObjectEventCallbackListFree(virObjectEventCallbackListPtr list)
virFreeCallback freecb = list->callbacks[i]->freecb;
if (freecb)
(*freecb)(list->callbacks[i]->opaque);
- VIR_FREE(list->callbacks[i]);
+ g_free(list->callbacks[i]);
}
- VIR_FREE(list->callbacks);
- VIR_FREE(list);
+ g_free(list->callbacks);
+ g_free(list);
}
@@ -494,7 +494,7 @@ virObjectEventQueueFree(virObjectEventQueuePtr queue)
return;
virObjectEventQueueClear(queue);
- VIR_FREE(queue);
+ g_free(queue);
}
static virObjectEventQueuePtr
diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c
index 4d7d685d6e..3341f5d83c 100644
--- a/src/conf/secret_conf.c
+++ b/src/conf/secret_conf.c
@@ -43,9 +43,9 @@ virSecretDefFree(virSecretDefPtr def)
if (def == NULL)
return;
- VIR_FREE(def->description);
- VIR_FREE(def->usage_id);
- VIR_FREE(def);
+ g_free(def->description);
+ g_free(def->usage_id);
+ g_free(def);
}
static int
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 0b78699589..df88a0bc72 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -105,7 +105,7 @@ virDomainSnapshotDiskDefFree(virDomainSnapshotDiskDefPtr disk)
return;
virDomainSnapshotDiskDefClear(disk);
- VIR_FREE(disk);
+ g_free(disk);
}
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 9aed602fd6..3f06fcaebf 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -448,15 +448,15 @@ virStorageVolDefFree(virStorageVolDefPtr def)
if (!def)
return;
- VIR_FREE(def->name);
- VIR_FREE(def->key);
+ g_free(def->name);
+ g_free(def->key);
for (i = 0; i < def->source.nextent; i++)
- VIR_FREE(def->source.extents[i].path);
- VIR_FREE(def->source.extents);
+ g_free(def->source.extents[i].path);
+ g_free(def->source.extents);
virStorageSourceClear(&def->target);
- VIR_FREE(def);
+ g_free(def);
}
@@ -497,7 +497,7 @@ void
virStoragePoolSourceFree(virStoragePoolSourcePtr source)
{
virStoragePoolSourceClear(source);
- VIR_FREE(source);
+ g_free(source);
}
@@ -507,16 +507,16 @@ virStoragePoolDefFree(virStoragePoolDefPtr def)
if (!def)
return;
- VIR_FREE(def->name);
+ g_free(def->name);
virStoragePoolSourceClear(&def->source);
- VIR_FREE(def->target.path);
- VIR_FREE(def->target.perms.label);
- VIR_FREE(def->refresh);
+ g_free(def->target.path);
+ g_free(def->target.perms.label);
+ g_free(def->refresh);
if (def->namespaceData && def->ns.free)
(def->ns.free)(def->namespaceData);
- VIR_FREE(def);
+ g_free(def);
}
diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c
index 6a32df15b7..7fd0df70a9 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -78,8 +78,8 @@ virStorageEncryptionFree(virStorageEncryptionPtr enc)
for (i = 0; i < enc->nsecrets; i++)
virStorageEncryptionSecretFree(enc->secrets[i]);
virStorageEncryptionInfoDefClear(&enc->encinfo);
- VIR_FREE(enc->secrets);
- VIR_FREE(enc);
+ g_free(enc->secrets);
+ g_free(enc);
}
static virStorageEncryptionSecretPtr
diff --git a/src/conf/storage_source_conf.c b/src/conf/storage_source_conf.c
index dab5e855f5..3eaf05fe52 100644
--- a/src/conf/storage_source_conf.c
+++ b/src/conf/storage_source_conf.c
@@ -159,7 +159,7 @@ virStorageNetHostDefFree(size_t nhosts,
for (i = 0; i < nhosts; i++)
virStorageNetHostDefClear(&hosts[i]);
- VIR_FREE(hosts);
+ g_free(hosts);
}
@@ -169,8 +169,8 @@ virStoragePermsFree(virStoragePermsPtr def)
if (!def)
return;
- VIR_FREE(def->label);
- VIR_FREE(def);
+ g_free(def->label);
+ g_free(def);
}
@@ -204,10 +204,10 @@ virStorageAuthDefFree(virStorageAuthDefPtr authdef)
if (!authdef)
return;
- VIR_FREE(authdef->username);
- VIR_FREE(authdef->secrettype);
+ g_free(authdef->username);
+ g_free(authdef->secrettype);
virSecretLookupDefClear(&authdef->seclookupdef);
- VIR_FREE(authdef);
+ g_free(authdef);
}
@@ -314,9 +314,9 @@ virStoragePRDefFree(virStoragePRDefPtr prd)
if (!prd)
return;
- VIR_FREE(prd->path);
- VIR_FREE(prd->mgralias);
- VIR_FREE(prd);
+ g_free(prd->path);
+ g_free(prd->mgralias);
+ g_free(prd);
}
@@ -511,7 +511,7 @@ virStorageSourceNVMeDefFree(virStorageSourceNVMeDefPtr def)
if (!def)
return;
- VIR_FREE(def);
+ g_free(def);
}
@@ -992,10 +992,10 @@ virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def)
if (!def)
return;
- VIR_FREE(def->pool);
- VIR_FREE(def->volume);
+ g_free(def->pool);
+ g_free(def->volume);
- VIR_FREE(def);
+ g_free(def);
}
diff --git a/src/conf/virdomaincheckpointobjlist.c
b/src/conf/virdomaincheckpointobjlist.c
index e09137b69b..a8025055c2 100644
--- a/src/conf/virdomaincheckpointobjlist.c
+++ b/src/conf/virdomaincheckpointobjlist.c
@@ -78,7 +78,7 @@ virDomainCheckpointObjListFree(virDomainCheckpointObjListPtr
checkpoints)
if (!checkpoints)
return;
virDomainMomentObjListFree(checkpoints->base);
- VIR_FREE(checkpoints);
+ g_free(checkpoints);
}
diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c
index 999f4a4152..7d639c4e01 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -229,7 +229,7 @@ virDomainMomentObjFree(virDomainMomentObjPtr moment)
VIR_DEBUG("obj=%p", moment);
virObjectUnref(moment->def);
- VIR_FREE(moment);
+ g_free(moment);
}
@@ -289,7 +289,7 @@ virDomainMomentObjListFree(virDomainMomentObjListPtr moments)
if (!moments)
return;
virHashFree(moments->objs);
- VIR_FREE(moments);
+ g_free(moments);
}
diff --git a/src/conf/virdomainsnapshotobjlist.c b/src/conf/virdomainsnapshotobjlist.c
index 7c2b63c5ae..4c7176b95e 100644
--- a/src/conf/virdomainsnapshotobjlist.c
+++ b/src/conf/virdomainsnapshotobjlist.c
@@ -101,7 +101,7 @@ virDomainSnapshotObjListFree(virDomainSnapshotObjListPtr snapshots)
if (!snapshots)
return;
virDomainMomentObjListFree(snapshots->base);
- VIR_FREE(snapshots);
+ g_free(snapshots);
}
diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c
index 244a168357..21ebbdb054 100644
--- a/src/conf/virnetworkportdef.c
+++ b/src/conf/virnetworkportdef.c
@@ -45,12 +45,12 @@ virNetworkPortDefFree(virNetworkPortDefPtr def)
if (!def)
return;
- VIR_FREE(def->ownername);
- VIR_FREE(def->group);
+ g_free(def->ownername);
+ g_free(def->group);
virNetDevBandwidthFree(def->bandwidth);
virNetDevVlanClear(&def->vlan);
- VIR_FREE(def->virtPortProfile);
+ g_free(def->virtPortProfile);
switch ((virNetworkPortPlugType)def->plugtype) {
case VIR_NETWORK_PORT_PLUG_TYPE_NONE:
@@ -58,11 +58,11 @@ virNetworkPortDefFree(virNetworkPortDefPtr def)
case VIR_NETWORK_PORT_PLUG_TYPE_NETWORK:
case VIR_NETWORK_PORT_PLUG_TYPE_BRIDGE:
- VIR_FREE(def->plug.bridge.brname);
+ g_free(def->plug.bridge.brname);
break;
case VIR_NETWORK_PORT_PLUG_TYPE_DIRECT:
- VIR_FREE(def->plug.direct.linkdev);
+ g_free(def->plug.direct.linkdev);
break;
case VIR_NETWORK_PORT_PLUG_TYPE_HOSTDEV_PCI:
@@ -73,7 +73,7 @@ virNetworkPortDefFree(virNetworkPortDefPtr def)
break;
}
- VIR_FREE(def);
+ g_free(def);
}
diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c
index 28c1c3938c..a3f90b86bb 100644
--- a/src/conf/virnwfilterbindingdef.c
+++ b/src/conf/virnwfilterbindingdef.c
@@ -36,13 +36,13 @@ virNWFilterBindingDefFree(virNWFilterBindingDefPtr def)
if (!def)
return;
- VIR_FREE(def->ownername);
- VIR_FREE(def->portdevname);
- VIR_FREE(def->linkdevname);
- VIR_FREE(def->filter);
+ g_free(def->ownername);
+ g_free(def->portdevname);
+ g_free(def->linkdevname);
+ g_free(def->filter);
virHashFree(def->filterparams);
- VIR_FREE(def);
+ g_free(def);
}
diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
index a6a5aa01c7..3157522eb2 100644
--- a/src/conf/virnwfilterobj.c
+++ b/src/conf/virnwfilterobj.c
@@ -99,7 +99,7 @@ virNWFilterObjFree(virNWFilterObjPtr obj)
virMutexDestroy(&obj->lock);
- VIR_FREE(obj);
+ g_free(obj);
}
@@ -109,8 +109,8 @@ virNWFilterObjListFree(virNWFilterObjListPtr nwfilters)
size_t i;
for (i = 0; i < nwfilters->count; i++)
virNWFilterObjFree(nwfilters->objs[i]);
- VIR_FREE(nwfilters->objs);
- VIR_FREE(nwfilters);
+ g_free(nwfilters->objs);
+ g_free(nwfilters);
}
--
2.29.2
3:54 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On Mon, Feb 01, 2021 at 01:27:20 -0500, Laine Stump wrote:
This patch takes on one set of examples of unnecessary use of
VIR_FREE() when g_free() is adequate - it modifies only vir*Free()
functions within the conf directory that take a single pointer and
free the object pointed to by that argument before returning. The
modification is to replace VIR_FREE() with g_free() for the object
itself *and* for all subordinate chunks of memory pointed to by that
object.
(NB: there are other functions that VIR_FREE subordinate memory of
objects that end up being freed before return (also sometimes with
VIR_FREE); I am purposefully ignoring those to reduce scope and focus
on a sub class where the pointlessness is obvious.)
Signed-off-by: Laine Stump <laine(a)redhat.com>
---
NB: After going through the experience a few days ago of needing to
more or less manually backport a patch due to a different patch
similar to this one touching the one function relevant to the desired
patch as well as many dozens of other functions (thus making it
impractical to backport that patch as a prerequisite), I contemplated
splitting this patch up all the way to 1 patch for every
function. That seemed extreme though, so I've left it as is. An
alternative, of course, is to just do nothing and leave everything as
VIR_FREE() (and I know there is sentiment in that direction, a bit
from me even :-P). I wonder though if we'll ever live up to the goals
listed in docs/glib-adoption.txt...
I reckon that there isn't much value in replacing VIR_FREE by g_free as
a separate cleanup step, unless we are about to remove VIR_FREE
altoghether and open-code it in places where it's required.
As of such I'm not in favor of such cleanup patch. There's quite lot of
churn and the technical justification is rather weak.
3:59 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On Mon, Feb 01, 2021 at 10:54:26AM +0100, Peter Krempa wrote:
On Mon, Feb 01, 2021 at 01:27:20 -0500, Laine Stump wrote:
> This patch takes on one set of examples of unnecessary use of
> VIR_FREE() when g_free() is adequate - it modifies only vir*Free()
> functions within the conf directory that take a single pointer and
> free the object pointed to by that argument before returning. The
> modification is to replace VIR_FREE() with g_free() for the object
> itself *and* for all subordinate chunks of memory pointed to by that
> object.
>
> (NB: there are other functions that VIR_FREE subordinate memory of
> objects that end up being freed before return (also sometimes with
> VIR_FREE); I am purposefully ignoring those to reduce scope and focus
> on a sub class where the pointlessness is obvious.)
>
> Signed-off-by: Laine Stump <laine(a)redhat.com>
> ---
>
> NB: After going through the experience a few days ago of needing to
> more or less manually backport a patch due to a different patch
> similar to this one touching the one function relevant to the desired
> patch as well as many dozens of other functions (thus making it
> impractical to backport that patch as a prerequisite), I contemplated
> splitting this patch up all the way to 1 patch for every
> function. That seemed extreme though, so I've left it as is. An
> alternative, of course, is to just do nothing and leave everything as
> VIR_FREE() (and I know there is sentiment in that direction, a bit
> from me even :-P). I wonder though if we'll ever live up to the goals
> listed in docs/glib-adoption.txt...
I reckon that there isn't much value in replacing VIR_FREE by g_free as
a separate cleanup step, unless we are about to remove VIR_FREE
altoghether and open-code it in places where it's required.
Removing entire of viralloc.h is the long term goal.
In most methods we remove alot of VIR_FREE usage by switching to
g_autofree. The virXXXXFree() methods though can't use g_autofree,
so ultimately they are were most remaining VIR_FREEs are going to
be. IOW, this change is a good step towards full elimination
of VIR_FREE.
As of such I'm not in favor of such cleanup patch. There's
quite lot of
churn and the technical justification is rather weak.
The justification is to get further towards fully eliminating VIR_FREE.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:05 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On Mon, Feb 01, 2021 at 09:59:57 +0000, Daniel Berrange wrote:
On Mon, Feb 01, 2021 at 10:54:26AM +0100, Peter Krempa wrote:
> On Mon, Feb 01, 2021 at 01:27:20 -0500, Laine Stump wrote:
> > This patch takes on one set of examples of unnecessary use of
> > VIR_FREE() when g_free() is adequate - it modifies only vir*Free()
> > functions within the conf directory that take a single pointer and
> > free the object pointed to by that argument before returning. The
> > modification is to replace VIR_FREE() with g_free() for the object
> > itself *and* for all subordinate chunks of memory pointed to by that
> > object.
> >
> > (NB: there are other functions that VIR_FREE subordinate memory of
> > objects that end up being freed before return (also sometimes with
> > VIR_FREE); I am purposefully ignoring those to reduce scope and focus
> > on a sub class where the pointlessness is obvious.)
> >
> > Signed-off-by: Laine Stump <laine(a)redhat.com>
> > ---
> >
> > NB: After going through the experience a few days ago of needing to
> > more or less manually backport a patch due to a different patch
> > similar to this one touching the one function relevant to the desired
> > patch as well as many dozens of other functions (thus making it
> > impractical to backport that patch as a prerequisite), I contemplated
> > splitting this patch up all the way to 1 patch for every
> > function. That seemed extreme though, so I've left it as is. An
> > alternative, of course, is to just do nothing and leave everything as
> > VIR_FREE() (and I know there is sentiment in that direction, a bit
> > from me even :-P). I wonder though if we'll ever live up to the goals
> > listed in docs/glib-adoption.txt...
>
> I reckon that there isn't much value in replacing VIR_FREE by g_free as
> a separate cleanup step, unless we are about to remove VIR_FREE
> altoghether and open-code it in places where it's required.
Removing entire of viralloc.h is the long term goal.
In most methods we remove alot of VIR_FREE usage by switching to
g_autofree. The virXXXXFree() methods though can't use g_autofree,
so ultimately they are were most remaining VIR_FREEs are going to
be. IOW, this change is a good step towards full elimination
of VIR_FREE.
> As of such I'm not in favor of such cleanup patch. There's quite lot of
> churn and the technical justification is rather weak.
The justification is to get further towards fully eliminating VIR_FREE.
In such case I'd strongly prefer if we replace all remaining usage of
VIR_FREE (even after this commit) right away to
g_clear_pointer(&ptr, g_free), whithout stretching the pain across
multiple spread-out refactoring steps.
I don't have a problem getting rid of it, I just don't want it to be
dragging out forever.
4:09 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On Mon, Feb 01, 2021 at 11:05:15AM +0100, Peter Krempa wrote:
On Mon, Feb 01, 2021 at 09:59:57 +0000, Daniel Berrange wrote:
> On Mon, Feb 01, 2021 at 10:54:26AM +0100, Peter Krempa wrote:
> > On Mon, Feb 01, 2021 at 01:27:20 -0500, Laine Stump wrote:
> > > This patch takes on one set of examples of unnecessary use of
> > > VIR_FREE() when g_free() is adequate - it modifies only vir*Free()
> > > functions within the conf directory that take a single pointer and
> > > free the object pointed to by that argument before returning. The
> > > modification is to replace VIR_FREE() with g_free() for the object
> > > itself *and* for all subordinate chunks of memory pointed to by that
> > > object.
> > >
> > > (NB: there are other functions that VIR_FREE subordinate memory of
> > > objects that end up being freed before return (also sometimes with
> > > VIR_FREE); I am purposefully ignoring those to reduce scope and focus
> > > on a sub class where the pointlessness is obvious.)
> > >
> > > Signed-off-by: Laine Stump <laine(a)redhat.com>
> > > ---
> > >
> > > NB: After going through the experience a few days ago of needing to
> > > more or less manually backport a patch due to a different patch
> > > similar to this one touching the one function relevant to the desired
> > > patch as well as many dozens of other functions (thus making it
> > > impractical to backport that patch as a prerequisite), I contemplated
> > > splitting this patch up all the way to 1 patch for every
> > > function. That seemed extreme though, so I've left it as is. An
> > > alternative, of course, is to just do nothing and leave everything as
> > > VIR_FREE() (and I know there is sentiment in that direction, a bit
> > > from me even :-P). I wonder though if we'll ever live up to the goals
> > > listed in docs/glib-adoption.txt...
> >
> > I reckon that there isn't much value in replacing VIR_FREE by g_free as
> > a separate cleanup step, unless we are about to remove VIR_FREE
> > altoghether and open-code it in places where it's required.
>
> Removing entire of viralloc.h is the long term goal.
>
> In most methods we remove alot of VIR_FREE usage by switching to
> g_autofree. The virXXXXFree() methods though can't use g_autofree,
> so ultimately they are were most remaining VIR_FREEs are going to
> be. IOW, this change is a good step towards full elimination
> of VIR_FREE.
>
> > As of such I'm not in favor of such cleanup patch. There's quite lot
of
> > churn and the technical justification is rather weak.
>
> The justification is to get further towards fully eliminating VIR_FREE.
In such case I'd strongly prefer if we replace all remaining usage of
VIR_FREE (even after this commit) right away to
g_clear_pointer(&ptr, g_free), whithout stretching the pain across
multiple spread-out refactoring steps.
I don't have a problem getting rid of it, I just don't want it to be
dragging out forever.
A bulk replacement isn't the right approach, because it will lead to
greater code churn. Much usage of VIR_FREE is better replaced by use
of g_autofree. What remains is better replaced by a simple g_free,
only a relatively small amount needs g_clear_pointer. Bulking replacing
everything with g_clear_pointer just means we'll need to replace it
yet again with the desired final solution. Incrementally converting
our codebase is just fine.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:13 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On a Monday in 2021, Daniel P. Berrangé wrote:
On Mon, Feb 01, 2021 at 11:05:15AM +0100, Peter Krempa wrote:
> On Mon, Feb 01, 2021 at 09:59:57 +0000, Daniel Berrange wrote:
> > On Mon, Feb 01, 2021 at 10:54:26AM +0100, Peter Krempa wrote:
> > > On Mon, Feb 01, 2021 at 01:27:20 -0500, Laine Stump wrote:
> > > > This patch takes on one set of examples of unnecessary use of
> > > > VIR_FREE() when g_free() is adequate - it modifies only vir*Free()
> > > > functions within the conf directory that take a single pointer and
> > > > free the object pointed to by that argument before returning. The
> > > > modification is to replace VIR_FREE() with g_free() for the object
> > > > itself *and* for all subordinate chunks of memory pointed to by that
> > > > object.
> > > >
> > > > (NB: there are other functions that VIR_FREE subordinate memory of
> > > > objects that end up being freed before return (also sometimes with
> > > > VIR_FREE); I am purposefully ignoring those to reduce scope and focus
> > > > on a sub class where the pointlessness is obvious.)
> > > >
> > > > Signed-off-by: Laine Stump <laine(a)redhat.com>
> > > > ---
> > > >
> > > > NB: After going through the experience a few days ago of needing to
> > > > more or less manually backport a patch due to a different patch
> > > > similar to this one touching the one function relevant to the desired
> > > > patch as well as many dozens of other functions (thus making it
> > > > impractical to backport that patch as a prerequisite), I contemplated
> > > > splitting this patch up all the way to 1 patch for every
> > > > function. That seemed extreme though, so I've left it as is. An
> > > > alternative, of course, is to just do nothing and leave everything as
> > > > VIR_FREE() (and I know there is sentiment in that direction, a bit
> > > > from me even :-P). I wonder though if we'll ever live up to the
goals
> > > > listed in docs/glib-adoption.txt...
> > >
> > > I reckon that there isn't much value in replacing VIR_FREE by g_free
as
> > > a separate cleanup step, unless we are about to remove VIR_FREE
> > > altoghether and open-code it in places where it's required.
> >
> > Removing entire of viralloc.h is the long term goal.
> >
> > In most methods we remove alot of VIR_FREE usage by switching to
> > g_autofree. The virXXXXFree() methods though can't use g_autofree,
> > so ultimately they are were most remaining VIR_FREEs are going to
> > be. IOW, this change is a good step towards full elimination
> > of VIR_FREE.
> >
> > > As of such I'm not in favor of such cleanup patch. There's quite
lot of
> > > churn and the technical justification is rather weak.
> >
> > The justification is to get further towards fully eliminating VIR_FREE.
>
> In such case I'd strongly prefer if we replace all remaining usage of
> VIR_FREE (even after this commit) right away to
> g_clear_pointer(&ptr, g_free), whithout stretching the pain across
> multiple spread-out refactoring steps.
>
> I don't have a problem getting rid of it, I just don't want it to be
> dragging out forever.
A bulk replacement isn't the right approach, because it will lead to
greater code churn. Much usage of VIR_FREE is better replaced by use
of g_autofree.
And dragging out the death of VIR_FREE makes it easier to search for
places that have not been converted to g_autofree yet.
Jano
What remains is better replaced by a simple g_free,
only a relatively small amount needs g_clear_pointer. Bulking replacing
everything with g_clear_pointer just means we'll need to replace it
yet again with the desired final solution. Incrementally converting
our codebase is just fine.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:20 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On Mon, Feb 01, 2021 at 10:09:25 +0000, Daniel Berrange wrote:
On Mon, Feb 01, 2021 at 11:05:15AM +0100, Peter Krempa wrote:
> On Mon, Feb 01, 2021 at 09:59:57 +0000, Daniel Berrange wrote:
[...]
> In such case I'd strongly prefer if we replace all remaining
usage of
> VIR_FREE (even after this commit) right away to
> g_clear_pointer(&ptr, g_free), whithout stretching the pain across
> multiple spread-out refactoring steps.
>
> I don't have a problem getting rid of it, I just don't want it to be
> dragging out forever.
A bulk replacement isn't the right approach, because it will lead to
greater code churn. Much usage of VIR_FREE is better replaced by use
of g_autofree. What remains is better replaced by a simple g_free,
only a relatively small amount needs g_clear_pointer. Bulking replacing
everything with g_clear_pointer just means we'll need to replace it
yet again with the desired final solution.
Well, that is extremely unlikely to be done soon there's a bit over 4k
VIR_FREE's left in various forgotten and unused parts of libvirt that
wasn't touched in a long time.
Spending time converting the use of those to g_free and g_autofree
manually would be in many cases a waste of time.
If we ever want to get rid of VIR_FREE in a timely manner it will IMO be
better to just replace it by the identical code, and let the cleanups
happen gradually and more localized in the parts people care about
actually.
Incrementally converting
our codebase is just fine.
I'd agree with that, but approach in this commit is somewhere betwen
incremental and massive conversion. It picks the low hanging uses, but
leaves the ones which will likely stay there for a very long time.
4:23 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On Mon, Feb 01, 2021 at 11:20:04AM +0100, Peter Krempa wrote:
On Mon, Feb 01, 2021 at 10:09:25 +0000, Daniel Berrange wrote:
> On Mon, Feb 01, 2021 at 11:05:15AM +0100, Peter Krempa wrote:
> > On Mon, Feb 01, 2021 at 09:59:57 +0000, Daniel Berrange wrote:
[...]
> > In such case I'd strongly prefer if we replace all remaining usage of
> > VIR_FREE (even after this commit) right away to
> > g_clear_pointer(&ptr, g_free), whithout stretching the pain across
> > multiple spread-out refactoring steps.
> >
> > I don't have a problem getting rid of it, I just don't want it to be
> > dragging out forever.
>
> A bulk replacement isn't the right approach, because it will lead to
> greater code churn. Much usage of VIR_FREE is better replaced by use
> of g_autofree. What remains is better replaced by a simple g_free,
> only a relatively small amount needs g_clear_pointer. Bulking replacing
> everything with g_clear_pointer just means we'll need to replace it
> yet again with the desired final solution.
Well, that is extremely unlikely to be done soon there's a bit over 4k
VIR_FREE's left in various forgotten and unused parts of libvirt that
wasn't touched in a long time.
Spending time converting the use of those to g_free and g_autofree
manually would be in many cases a waste of time.
If we ever want to get rid of VIR_FREE in a timely manner it will IMO be
better to just replace it by the identical code, and let the cleanups
happen gradually and more localized in the parts people care about
actually.
>Incrementally converting
> our codebase is just fine.
I'd agree with that, but approach in this commit is somewhere betwen
incremental and massive conversion. It picks the low hanging uses, but
leaves the ones which will likely stay there for a very long time.
I'd say it picks the cases which are clearly correct to convert directly
to g_free, and leaves the cases which are likely going to need to use
either g_auto* or g_clear_pointer. IMHO this is the correct way to do
the conversion.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:25 p.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On 2/1/21 5:23 AM, Daniel P. Berrangé wrote:
On Mon, Feb 01, 2021 at 11:20:04AM +0100, Peter Krempa wrote:
> On Mon, Feb 01, 2021 at 10:09:25 +0000, Daniel Berrange wrote:
>> On Mon, Feb 01, 2021 at 11:05:15AM +0100, Peter Krempa wrote:
>>> On Mon, Feb 01, 2021 at 09:59:57 +0000, Daniel Berrange wrote:
> [...]
>
>>> In such case I'd strongly prefer if we replace all remaining usage of
>>> VIR_FREE (even after this commit) right away to
>>> g_clear_pointer(&ptr, g_free), whithout stretching the pain across
>>> multiple spread-out refactoring steps.
>>>
>>> I don't have a problem getting rid of it, I just don't want it to be
>>> dragging out forever.
>> A bulk replacement isn't the right approach, because it will lead to
>> greater code churn. Much usage of VIR_FREE is better replaced by use
>> of g_autofree. What remains is better replaced by a simple g_free,
>> only a relatively small amount needs g_clear_pointer. Bulking replacing
>> everything with g_clear_pointer just means we'll need to replace it
>> yet again with the desired final solution.
> Well, that is extremely unlikely to be done soon there's a bit over 4k
> VIR_FREE's left in various forgotten and unused parts of libvirt that
> wasn't touched in a long time.
>
> Spending time converting the use of those to g_free and g_autofree
> manually would be in many cases a waste of time.
>
> If we ever want to get rid of VIR_FREE in a timely manner it will IMO be
> better to just replace it by the identical code, and let the cleanups
> happen gradually and more localized in the parts people care about
> actually.
>
>> Incrementally converting
>> our codebase is just fine.
> I'd agree with that, but approach in this commit is somewhere betwen
> incremental and massive conversion. It picks the low hanging uses, but
> leaves the ones which will likely stay there for a very long time.
I'd say it picks the cases which are clearly correct to convert directly
to g_free, and leaves the cases which are likely going to need to use
either g_auto* or g_clear_pointer. IMHO this is the correct way to do
the conversion.
Speaking of "other cases" - here are three classes of
"non-g_autofreeable" VIR_FREE() that I see a lot of when looking in the
conf directory (which unsurprisingly has 717 of the 4k+ uses of
VIR_FREE(). If anyone has ideas/opinions on these, I wouldn't mind
hearing it:
1) calling VIR_FREE() in a virBlahDispose() function to free subordinate
objectspointed to by the objec being disposed of. Is there any reason to
*not* convert those VIR_FREEs to g_free()? There's nothing that would
ever look at the contents of an object after its vir*Dispose() function
has been called is there?
2) calling VIR_FREE() in a virBlahClear() function. Technically these
functions *do* need to set the pointer to NULL, because well, it says it
right there in the name of the function! However, in several of the
instances I checked, the only caller to the vir*Clear() function was a
vir*Free() function that was cleaning up its subordinate objects prior
to freeing itself. Usually those subordinate objects are contained in
the larger object (rather than pointed to) in the name of efficiency
(less calls to mallo... er I mean g_new0()). Do you think there's any
point to, e.g. turning these "virBlah item" members into ",virBlahPtr
item" so they are gotten rid of with virBlahFree() instead of
virBlahClear()? Or is this one of the cases where it's definitely proper
to use g_clear_pointer()
3) g_autofree pointers called "tmp", "str", "nodes", and
probably some
others that are re-used several times in a function, and are VIR_FREEd
after each use. Aside from breaking the rule/guideline that you should
never explicitly free an g_autofree pointer, they by definition won't
simply go away by converting to g_autofree - they've already been
converted! I can see two simple ways of eliminating these: 1) make
multiple g_autofree char *'s in each function, once for each usage (will
the compiler be smart enough to optimize these into a single pointer if
the usage doesn't overlap?), or 2) switch to using g_clear_pointer() (is
*that* also frowned upon for pointers that are already g_autofree?)
4) I know I said three, but there are also several examples of
VIR_FREE() being called in a loop on the individual items in an array
prior to freeing the array (see every example of calling
virBlkioDeviceArrayClear(), for example). I don't think anyone has spent
any time looking into converting things to use GArray and GPtrArray, but
I suppose that's the best way to get rid of those...
4:17 a.m.
New subject: [PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions
On Tue, Feb 02, 2021 at 12:25:40AM -0500, Laine Stump wrote:
On 2/1/21 5:23 AM, Daniel P. Berrangé wrote:
> On Mon, Feb 01, 2021 at 11:20:04AM +0100, Peter Krempa wrote:
> > On Mon, Feb 01, 2021 at 10:09:25 +0000, Daniel Berrange wrote:
> > > On Mon, Feb 01, 2021 at 11:05:15AM +0100, Peter Krempa wrote:
> > > > On Mon, Feb 01, 2021 at 09:59:57 +0000, Daniel Berrange wrote:
> > [...]
> >
> > > > In such case I'd strongly prefer if we replace all remaining
usage of
> > > > VIR_FREE (even after this commit) right away to
> > > > g_clear_pointer(&ptr, g_free), whithout stretching the pain
across
> > > > multiple spread-out refactoring steps.
> > > >
> > > > I don't have a problem getting rid of it, I just don't want
it to be
> > > > dragging out forever.
> > > A bulk replacement isn't the right approach, because it will lead to
> > > greater code churn. Much usage of VIR_FREE is better replaced by use
> > > of g_autofree. What remains is better replaced by a simple g_free,
> > > only a relatively small amount needs g_clear_pointer. Bulking replacing
> > > everything with g_clear_pointer just means we'll need to replace it
> > > yet again with the desired final solution.
> > Well, that is extremely unlikely to be done soon there's a bit over 4k
> > VIR_FREE's left in various forgotten and unused parts of libvirt that
> > wasn't touched in a long time.
> >
> > Spending time converting the use of those to g_free and g_autofree
> > manually would be in many cases a waste of time.
> >
> > If we ever want to get rid of VIR_FREE in a timely manner it will IMO be
> > better to just replace it by the identical code, and let the cleanups
> > happen gradually and more localized in the parts people care about
> > actually.
> >
> > > Incrementally converting
> > > our codebase is just fine.
> > I'd agree with that, but approach in this commit is somewhere betwen
> > incremental and massive conversion. It picks the low hanging uses, but
> > leaves the ones which will likely stay there for a very long time.
> I'd say it picks the cases which are clearly correct to convert directly
> to g_free, and leaves the cases which are likely going to need to use
> either g_auto* or g_clear_pointer. IMHO this is the correct way to do
> the conversion.
Speaking of "other cases" - here are three classes of
"non-g_autofreeable"
VIR_FREE() that I see a lot of when looking in the conf directory (which
unsurprisingly has 717 of the 4k+ uses of VIR_FREE(). If anyone has
ideas/opinions on these, I wouldn't mind hearing it:
1) calling VIR_FREE() in a virBlahDispose() function to free subordinate
objectspointed to by the objec being disposed of. Is there any reason to
*not* convert those VIR_FREEs to g_free()? There's nothing that would ever
look at the contents of an object after its vir*Dispose() function has been
called is there?
Yep, a virBlahDispose function should be considered the same as
a virBlahFree function. The only difference is that in the former
case the final g_free(blah) of the struct itself is done for you.
2) calling VIR_FREE() in a virBlahClear() function. Technically
these
functions *do* need to set the pointer to NULL, because well, it says it
right there in the name of the function! However, in several of the
instances I checked, the only caller to the vir*Clear() function was a
vir*Free() function that was cleaning up its subordinate objects prior to
freeing itself. Usually those subordinate objects are contained in the
larger object (rather than pointed to) in the name of efficiency (less calls
to mallo... er I mean g_new0()). Do you think there's any point to, e.g.
turning these "virBlah item" members into ",virBlahPtr item" so they
are
gotten rid of with virBlahFree() instead of virBlahClear()? Or is this one
of the cases where it's definitely proper to use g_clear_pointer()
A simpler option might be to just put a "memset(blah, 0, sizeof(blah))"
at the end of the virBlahClear function so that we splatter the entire
struct at once.
3) g_autofree pointers called "tmp", "str",
"nodes", and probably some
others that are re-used several times in a function, and are VIR_FREEd after
each use. Aside from breaking the rule/guideline that you should never
explicitly free an g_autofree pointer, they by definition won't simply go
away by converting to g_autofree - they've already been converted! I can see
two simple ways of eliminating these: 1) make multiple g_autofree char *'s
in each function, once for each usage (will the compiler be smart enough to
optimize these into a single pointer if the usage doesn't overlap?), or 2)
switch to using g_clear_pointer() (is *that* also frowned upon for pointers
that are already g_autofree?)
I think in general we probably prefer to *not* refuse variables multiple
times. Where we do though, I'd say g_clear_pointer is the semantically
correct approach.
4) I know I said three, but there are also several examples of
VIR_FREE()
being called in a loop on the individual items in an array prior to freeing
the array (see every example of calling virBlkioDeviceArrayClear(), for
example). I don't think anyone has spent any time looking into converting
things to use GArray and GPtrArray, but I suppose that's the best way to get
rid of those...
Yep, but using g_free directly is fine here in the meanwhile.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
3:19 a.m.
On 2/1/21 7:27 AM, Laine Stump wrote:
Another set of small changes that started out with me noticing
something I didn't like, and organically leading down a Kerouacian
road to nowhere and everywhere.
Most of them are related to eliminating unnecessary nullifying of
objects that are about to be free'd anyway, but patch 2 is obsolete
dead code, and patch 1 is a theoretical "failure with no registered
error message".
Laine Stump (8):
log error if virConnectCacheOnceInit() fails
hostdevmgr: remove unneeded oldStateDir
conf: replace g_clear_pointer(..., g_hash_table_unref) with
virHashFree()
conf: fix arg to virDomainPCIAddressSetExtensionFree()
conf: don't bother setting pointers to NULL in vir*Free() functions
conf: eliminate pointless setting of interface model
util: rename virStorageEncryptionInfoDefFree()
conf: replace VIR_FREE() with g_free() in vir*Free() functions
src/conf/capabilities.c | 8 +-
src/conf/cpu_conf.c | 8 +-
src/conf/device_conf.c | 2 +-
src/conf/domain_addr.c | 32 +--
src/conf/domain_capabilities.c | 6 +-
src/conf/domain_conf.c | 304 +++++++++++++-------------
src/conf/interface_conf.c | 32 +--
src/conf/network_conf.c | 18 +-
src/conf/node_device_conf.c | 105 +++++----
src/conf/nwfilter_conf.c | 22 +-
src/conf/nwfilter_params.c | 16 +-
src/conf/object_event.c | 12 +-
src/conf/secret_conf.c | 6 +-
src/conf/snapshot_conf.c | 2 +-
src/conf/storage_conf.c | 22 +-
src/conf/storage_encryption_conf.c | 8 +-
src/conf/storage_source_conf.c | 26 +--
src/conf/virdomaincheckpointobjlist.c | 2 +-
src/conf/virdomainmomentobjlist.c | 4 +-
src/conf/virdomainsnapshotobjlist.c | 2 +-
src/conf/virnetworkportdef.c | 12 +-
src/conf/virnwfilterbindingdef.c | 10 +-
src/conf/virnwfilterobj.c | 6 +-
src/driver.c | 22 +-
src/hypervisor/virhostdev.c | 60 +----
src/hypervisor/virhostdev.h | 6 +-
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 4 +-
src/qemu/qemu_hostdev.c | 3 +-
tests/virhostdevtest.c | 10 +-
30 files changed, 363 insertions(+), 409 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1389
days inactive
1390
days old
26 comments
5 participants
participants (5)
-
Daniel P. Berrangé -
Ján Tomko -
Laine Stump -
Michal Privoznik -
Peter Krempa