[PATCH 8/8] conf: replace VIR_FREE() with g_free() in vir*Free() functions

[Daniel P. Berrangé]

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 :|