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