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.