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

[Peter Krempa]

On Wed, Mar 10, 2021 at 18:41:35 +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.

Yup, we could possibly automatically fix the variable declarations to be
one-per-line at the beginning.

> 
> 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);

This is weird ... 

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

.. so this is the right solution for that case.