On Wed, Mar 10, 2021 at 06:41:35PM +0100, Michal Privoznik wrote: > On 3/10/21 4:36 PM, Michal Privoznik wrote: > > On 3/9/21 6:44 PM, Daniel P. Berrangé wrote: > > > One of the conventions we have had since the early days of libvirt is > > > that every struct typedef, has a corresponding "Ptr" typedef too. > > > > > > For example > > > > > > typedef struct _virDomainDef virDomainDef; > > > typedef virDomainDef *virDomainDefPtr; > > > > > > Periodically someone has questioned what the purpose of these Ptr > > > typedefs is, and we've not had an compelling answer, other than > > > that's what we've always done. > > > > > > > One possible upside is that it allows for less scary function headers, > > for instance: > > > > virDomainChrGetDomainPtrsInternal(virDomainDefPtr vmdef, > > virDomainChrDeviceType type, > > virDomainChrDefPtr ***arrPtr, > > size_t **cntPtr); > > > > Three levels of dereference don't look as bad as four. > > But yeah, I agree we should drop them. I wonder if cocci can help here. > > Or even plain in-place sed, except we'd have to be careful with > > statements like: > > > > virSomethingPtr a, b; > > > > where both @a and @b are pointers, and plain substitution would break it: > > > > virSomething *a, b; > > > > (here only @a is poitner, ofc). > > > > But since we have Peter nagging about this kind of variable declaration, > > it's not something one couldn't fix by hand. And in fact, whilst we're > > at it we could declare each variable at its own line. > > Okay, another problem: how does this g_auto() and similar work? I mean, now > we have: > > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDefPtr, virSysinfoDefFree, NULL); > > and it works fine. But obviously either of: > > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef *, ...); > G_DEFINE_AUTO_CLEANUP_FREE_FUNC(virSysinfoDef, ...); > > is plain wrong. Maybe we can switch to g_autoptr() instead.
Yes, we should be using g_autoptr even today, because g_auto is not for this use case: https://developer.gnome.org/glib/2.64/glib-Miscellaneous-Macros.html#g-auto "This is meant to be used with stack-allocated structures and non-pointer types. For the (more commonly used) pointer version, see g_autoptr()." 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 :|
