[libvirt PATCH 0/9] eliminate (almost) all VIR_FREE in esx directory

Michal Privoznik mprivozn at redhat.com
Mon Feb 15 10:17:34 UTC 2021


On 2/12/21 11:07 PM, Laine Stump wrote:
> 142 more down, 3084 to go :-)
> 
> This goes through all the standard methods of eliminating VIR_FREE -
> first converting as many pointers as possible to g_autofree, then
> converting a few more *Free* functions (that were more questionable
> than previous Frees) to use g_free, then some trivial refactoring. And
> finally at the end a few stragglers that really do need the pointers
> cleared were changed to g_clear_pointer(x, g_free).
> 
> 
> A couple of issues:
> 
> 1) There are two definitions of a macro called ESX_VI__TEMPLATE__FREE
> that generate functions with names in the form of
> "esxVI_.*_Free". These generated functions were doing
> "VIR_FREE(*ptrptr)" at the end; because of the "*", the caller's
> pointer would be cleared out (not just the local copy). There are
> something over 400 calls to these functions, and all but 10 or so that
> I audited will never reference the pointer after return from
> esxVI_Blah_Free(). But there are several that do. Since I'm unfamiliar
> with this code and don't want to risk breaking it, I opted to just use
> g_clear_pointer(ptrptr, g_free), thus preserving current behavior
> exactly.
> 
> 2) There are still 7 instances of VIR_FREE in the esx directory. All 7
> of them are in loops that are clearing out an array of names prior to
> returning failure, and from a quick glance it looks like there are at
> least a few places where it is important to clear the array entries. But rather than brute force convert to using g_clear_pointer in the loop, I figured someone may come up with an elegant translation to use GArray or GPtrArray instead, so I'm leaving them for now.
> 
> 
> 
> Laine Stump (9):
>    esx: use g_autofree for char* where it is trivially possible
>    esx: use g_autofree when made possible by reducing scope
>    esx: fix memory leak by switching to g_autofree
>    esx: switch VIR_FREE->g_free in esx*Free*()
>    esx: use g_steal_pointer+g_autofree on return value
>    esx: reorder code to avoid need to VIR_FREE mimeType
>    esx: switch VIR_FREE->g_free when the pointer will immediately go out
>      of scope
>    esx: eliminate unnecessary cleanup: labels and result variables
>    esx: replace some VIR_FREE with g_clear_pointer(x, g_free)
> 
>   src/esx/esx_driver.c                | 189 +++++++++-------------------
>   src/esx/esx_network_driver.c        |   4 +-
>   src/esx/esx_storage_backend_iscsi.c |  16 +--
>   src/esx/esx_storage_backend_vmfs.c  | 150 +++++++---------------
>   src/esx/esx_stream.c                |  11 +-
>   src/esx/esx_util.c                  |  41 +++---
>   src/esx/esx_vi.c                    | 111 ++++++----------
>   src/esx/esx_vi_methods.c            |   3 +-
>   src/esx/esx_vi_types.c              |  18 +--
>   9 files changed, 179 insertions(+), 364 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

Michal




More information about the libvir-list mailing list