[PATCH 3/8] conf: replace g_clear_pointer(..., g_hash_table_unref) with virHashFree()
Peter Krempa
pkrempa at redhat.com
Mon Feb 1 09:47:47 UTC 2021
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 at 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.
More information about the libvir-list
mailing list