Re: [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.