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