[RFC] qemu: convert DomainLogContext class to use GObject

[Daniel P. Berrangé]

On Wed, Mar 11, 2020 at 10:26:24AM +0100, Ján Tomko wrote:
> On a Wednesday in 2020, Peter Krempa wrote:
> > On Tue, Mar 10, 2020 at 17:30:28 +0000, Daniel Berrange wrote:
> > > On Tue, Mar 10, 2020 at 06:20:49PM +0100, Ján Tomko wrote:
> > > > On a Tuesday in 2020, Gaurav Agrawal wrote:
> > > > > ---
> > > > > src/qemu/qemu_domain.c  | 36 ++++++++++++++++++++----------------
> > > > > src/qemu/qemu_domain.h  |  6 ++++--
> > > > > src/qemu/qemu_process.c |  4 ++--
> > > > > 3 files changed, 26 insertions(+), 20 deletions(-)
> > > > >
> > > >
> > > > [...]
> > > >
> > > > > @@ -10632,7 +10635,8 @@ qemuDomainLogContextPtr qemuDomainLogContextNew(virQEMUDriverPtr driver,
> > > > >     return ctxt;
> > > > >
> > > > >  error:
> > > > > -    virObjectUnref(ctxt);
> > > > > +    if (ctxt)
> > > > > +        g_object_unref(ctxt);
> > > >
> > > > g_object_unref is safe to call with a NULL argument, the "if (ctxt)"
> > > > check is not needed here.
> > > 
> > > I'm not so sure on that.
> > > 
> 
> The code itself does check for NULL, it's hidden in the
>   g_return_if_fail (G_IS_OBJECT (object));
> call which boils down to
>   g_type_check_instance_is_fundamentally_a

Yes, I think I can see this now, but I wonder if that's an intentionale
guarantee or not.

> Even though the documentation does not mention NULL is safe, it has to be, since it's
> used as a cleanup function in gobject/gobject-autocleanups.h

The G_DEFINE_AUTOPTR_CLEANUP_FUNC macro expands to code which
has an explicit "if(ptr)" check in it, so the actual func
doesn't require this.

> > >    https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html#g-clear-object
> > 
> > I'd prefer we agree on using this one globally on the same basis we had
> > for using VIR_FREE even on cleanup paths.
> 
> Sounds good. Most of the unrefs will be handled by g_auto anyway.

Yep, makes sense in majority of cases.

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 :|