[libvirt] [GSoC] Code design for scalar and external types

[Pavel Hrdina]

On Wed, Jun 13, 2018 at 12:54:54PM +0200, Andrea Bolognani wrote:
> On Mon, 2018-06-11 at 12:12 +0200, Pavel Hrdina wrote:
> > Another issue that we need to take into account is that the external
> > free functions might not be 'NULL' safe which we need to somehow ensure
> > if they will be used with attribute cleanup.
> > 
> > The original design is probably wrong and was heavily counting on the
> > existing libvirt implementation.  We might need to update the design
> > to make it the similar as GLib:
> > 
> > #define VIR_AUTOPTR_FUNC_NAME(type) virAutoPtr##type
> > #define VIR_AUTOPTR_TYPE_NAME(type) ##typeAutoPtr
> > 
> > #define VIR_AUTOPTR(type) \
> >     __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) VIR_AUTOPTR_TYPE_NAME(type)
> > 
> > #define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> >     typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
> >     static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
> >     { \
> >         if (*_ptr) \
> >             (func)(*_ptr); \
> >     }
> > 
> > The usage would like this:
> > 
> > VIR_DEFINE_AUTOPTR_FUNC(virAuthConfig, virAuthConfigFree)
> > 
> > VIR_AUTOPTR(virAuthConfig) authConfig = NULL;
> > 
> > 
> > That way we can use any existing free function regardless whether it
> > checks if the variable is NULL or not and without the need to manually
> > define new type.
> > 
> > The only exception would be the virStringList where we would have to use
> > the different type.
> 
> As danpb (CC'd) mentioned in [1], having the vir*Free() function
> take a double pointer is good because it allows it to also set it
> to NULL. I will add that any vir*Free() function that doesn't deal
> gracefully with being passed a NULL pointer is already buggy and
> should be fixed.
> 
> This new proposal would not deal with either, and it seems to me
> like it would be preferable to stick with the original one and, as
> a prerequisite, change all vir*Free() functions so that they follow
> the design outlined above, as doing so would benefit all code that
> still needs to perform manual memory management as well.
> 
> Are there any known issues that would make such an approach not
> feasible? Would it expand the scope of the project too much?

You've missed the whole reason for this new proposal.  Yes, we can fix
our internal vir*Free() functions but this autofree functionality would
be nice also for types from external libraries where the types have
their own free function and that we cannot fix.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180613/0094db03/attachment-0001.sig>