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@xxxxxxxxxx>
---
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
