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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
> 
> 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.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux