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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux