Re: [PATCH 4/4] ch: Don't leak ch_driver->chCaps
On Tue, Nov 28, 2023 at 11:43:11AM -0600, Praveen K Paladugu wrote:
On 11/28/2023 9:59 AM, Michal Privoznik wrote:
> During CH driver initialization (chStateInitialize()) the
> driver's capabilities bitmap is allocated
> (virCHCapsInitCHVersionCaps()), but corresponding free call is
> missing in chStateCleanup().
>
> And while at it, reorder calls to virObjectUnref() inside of
> chStateCleanup() to be the reverse order of that in
> chStateInitialize() so that it's easier to spot missing
> free/unref call.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/ch/ch_driver.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
> index bd271fc0ee..96de5044ac 100644
> --- a/src/ch/ch_driver.c
> +++ b/src/ch/ch_driver.c
> @@ -850,10 +850,11 @@ static int chStateCleanup(void)
> if (ch_driver == NULL)
> return -1;
> - virObjectUnref(ch_driver->domains);
> + virBitmapFree(ch_driver->chCaps);
`chCaps` is implemented as `g_autoptr`. Is this free required here because
`chCaps` never goes out of scope because it is used in `ch_driver`?
If that is the case, is there any value is having `chCaps` as `g_autoptr`?
In this context 'chCaps' is a struct field, and so we need to free any
fields when the struct (ch_driver) is freed.
The g_autoptr is useful for variables that are only alive / referenced
for the duration of a method (or inner code block scope).
Our best practice is that all data types have g_autoptr support no
matter whether the current code actually needs it or not.
> + virObjectUnref(ch_driver->config);
> virObjectUnref(ch_driver->xmlopt);
> virObjectUnref(ch_driver->caps);
> - virObjectUnref(ch_driver->config);
> + virObjectUnref(ch_driver->domains);
> virMutexDestroy(&ch_driver->lock);
> g_clear_pointer(&ch_driver, g_free);
--
Regards,
Praveen
_______________________________________________
Devel mailing list -- devel(a)lists.libvirt.org
To unsubscribe send an email to devel-leave(a)lists.libvirt.org
With 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 :|