Re: [libvirt] [GSoC] Code design for scalar and external types
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?
[1] https://www.redhat.com/archives/libvir-list/2018-June/msg00033.html
--
Andrea Bolognani / Red Hat / Virtualization