On Fri, May 15, 2020 at 02:34:01PM +0200, Michal Privoznik wrote: > On 5/13/20 1:56 PM, Rafael Fonseca wrote: > > This patch series convert various simple instances of virObject to a > > GObject equivalent. > > I'm sorry upfront for misusing your patchset and I'm also sorry for bringing > this up again. > > I think we need to step back and re-evaluate whether this is worth it. > GObject is horrible and frightening part of GLib. Not only one has to define > empty functions, but we can't mix virObject and GObject. For instance: > virObjectRef() called over GObject will corrupt memory. > > Worse, there is no way to check whether your patches converted everything > (and clearly they did not because I see couple of tests crashing; or maybe > they did at the time of send but now the code has changed - anyway, point > proven). > > I started reviewing and stopped at the first patch realizing, I have no idea > whether you converted every virObjectRef()/virObjectUnref() with > corresponding glib call. > > I also wanted to write a cocci spatch that might at least identify places > where that is happening, but apparently my spatch skills are poor. > > Does anybody have an idea how to verify these patches? My preferred option was to make virObject be a subclass of GObject. I tried this initially but hit a key problem - g_object_unref is void and virObjectUnref returns bool - true if any refs still exist. We rely on that for the virConnectPtr and virFDStream objects. I couldn't come up with a way to solve that at the time, but I've just had another think and believe we can solve it using a thread local set by the finalize. So I'll have another go at doing this inheritance. 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 :|
