RFC: do we want/need the "Ptr" typedefs for internal code ?

Daniel P. Berrangé berrange at redhat.com
Wed Mar 10 17:57:02 UTC 2021


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




More information about the libvir-list mailing list