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