Re: [PATCH 4/4] ch: Don't leak ch_driver->chCaps

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

 



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@xxxxxxxxxx>
> > ---
> >   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@xxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx

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 :|
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[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