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?