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