On Tue, Oct 15, 2019 at 10:51:39AM -0300, Daniel Henrique Barboza wrote: > > > On 10/15/19 9:55 AM, Daniel P. Berrangé wrote: > > On Tue, Oct 15, 2019 at 09:42:45AM -0300, Daniel Henrique Barboza wrote: > > > I was hoping to quickly re-send the qemu_driver cleanups I've > > > sent some time ago, now using Glib. I started by attempting to > > > change the first VIR_AUTOUNREF() call in qemu_driver.c to > > > g_autoptr(), which happens to be a virStorageSourcePtr type, > > > then I realized that it wasn't that simple. > > It should be that simple with this commit: > > > > commit 667ff797e8eb8d82f30ab430216a8d2eef6b915a > > Author: Daniel P. Berrangé <berrange@xxxxxxxxxx> > > Date: Fri Oct 4 17:14:10 2019 +0100 > > > > src: add support for g_autoptr with virObject instances > > > > we should be able to use g_autoptr for any virObject, without > > having to lock-step convert to GObject. > > > > What actual problem did you find ? > > I failed to notice this commit. Just tried it again and it worked. > > What happened yesterday was that I attempted to do a simple > VIR_AUTOUNREF -> g_autopt replace, faced compile errors and then, since > I didn't notice this commit about, I assumed "I guess I need to convert > this guy to GObject". > > In fact, the compile error happened because g_autoptr() does not operate > with a 'Ptr' type - something that I learned only during the conversion > process. Yeah, you need to drop the 'Ptr' suffix in the type name when converting to g_autoptr, as it adds the pointer itself. > > > This is being sent as RFC because x-I am aware that docs/hacking.html > > > mentions that we shouldn't mix up certain GLib macros with Libvirt > > > ones, thus I am uncertain of whether I have messed up or not. > > > 'make check' works, did a few sanity checks with libvirtd as > > > well. > > Yes, the need to not mix g_auto* with VIR_AUTO*, is why I did commit > > 667ff797e8eb8d82f30ab430216a8d2eef6b915a to let you use g_autoptr > > with virObject, without first converting to GObject. > > What if there are other object types in the same function using the VIR > macros? > For example, inside qemu_driver.c: qemuDomainBlockCopyCommon: > > > VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = > virQEMUDriverGetConfig(driver); > const char *format = NULL; > bool mirror_reuse = !!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT); > bool mirror_shallow = !!(flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW); > bool existing = mirror_reuse; > qemuBlockJobDataPtr job = NULL; > VIR_AUTOUNREF(virStorageSourcePtr) mirror = mirrorsrc; > bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); > bool mirror_initialized = false; > VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; > VIR_AUTOPTR(qemuBlockStorageSourceChainData) crdata = NULL; > > > Let's say that I change the virStorageSourcePtr up there to > > g_autoptr(virStorageSource) mirror = mirrorsrc; > > > As long as there are no VIR macros acting in the 'mirror' variable, is it to > use g_autoptr > there even when everyone else is using VIR_AUTO* macros? You should change all variables in the method at the same time. Both the VIR_AUTOUNEF calls here can use g_autoptr, as can the two VIR_AUTOPTR calls. 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
