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

Peter Krempa pkrempa at redhat.com
Mon Feb 1 09:54:26 UTC 2021


On Mon, Feb 01, 2021 at 01:27:20 -0500, Laine Stump wrote:
> This patch takes on one set of examples of unnecessary use of
> VIR_FREE() when g_free() is adequate - it modifies only vir*Free()
> functions within the conf directory that take a single pointer and
> free the object pointed to by that argument before returning. The
> modification is to replace VIR_FREE() with g_free() for the object
> itself *and* for all subordinate chunks of memory pointed to by that
> object.
> 
> (NB: there are other functions that VIR_FREE subordinate memory of
> objects that end up being freed before return (also sometimes with
> VIR_FREE); I am purposefully ignoring those to reduce scope and focus
> on a sub class where the pointlessness is obvious.)
> 
> Signed-off-by: Laine Stump <laine at redhat.com>
> ---
> 
> NB: After going through the experience a few days ago of needing to
> more or less manually backport a patch due to a different patch
> similar to this one touching the one function relevant to the desired
> patch as well as many dozens of other functions (thus making it
> impractical to backport that patch as a prerequisite), I contemplated
> splitting this patch up all the way to 1 patch for every
> function. That seemed extreme though, so I've left it as is. An
> alternative, of course, is to just do nothing and leave everything as
> VIR_FREE() (and I know there is sentiment in that direction, a bit
> from me even :-P). I wonder though if we'll ever live up to the goals
> listed in docs/glib-adoption.txt...

I reckon that there isn't much value in replacing VIR_FREE by g_free as
a separate cleanup step, unless we are about to remove VIR_FREE
altoghether and open-code it in places where it's required.

As of such I'm not in favor of such cleanup patch. There's quite lot of
churn and the technical justification is rather weak.




More information about the libvir-list mailing list