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

Hi, It took me longer to sit down and write this mail but here it goes. There was a lot of ideas about the macro design and it's easy to get lost in all the designs. So we agreed on this form: # define VIR_AUTOPTR_FUNC_NAME(type) virAutoPtr##type # define VIR_AUTOCLEAR_FUNC_NAME(type) virAutoClear##type # define VIR_DEFINE_AUTOPTR_FUNC(type, func) \ static inline void VIR_AUTOPTR_FUNC_NAME(type)(type *_ptr) \ { \ (func)(*_ptr); \ } # define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \ static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \ { \ (func)(_ptr); \ } # define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type # define VIR_AUTOPTR(type) \ __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) type # define VIR_AUTOCLEAR(type) \ __attribute__((cleanup(VIR_AUTOCLEAR_FUNC_NAME(type)))) type but it turned out the it's not sufficient for several reasons: - there is no simple way ho to free list of strings (char **) - this will not work for external types with their own free function In order to solve these two issues there were some ideas. 1. How to handle list of strings We have virStringListFree() function in order to free list of strings, the question is how to use it with these macros: - Create a new typedef for list of strings and use VIR_AUTOPTR: typedef char **virStringList; VIR_DEFINE_AUTOPTR_FUNC(virStringList, virStringListFree); VIR_AUTOPTR(virStringList) list = NULL; The benefit of this solution is that it works with current design and we only need to create single typedef. - Overload VIR_AUTOFREE macro by making it variadic # define _VIR_AUTOFREE_0(type) __attribute__((cleanup(virFree))) type # define _VIR_AUTOFREE_1(type, func) __attribute__((cleanup(func))) type # define _VIR_AUTOFREE_OVERLOADER(_1, _2, NAME, ...) NAME # define VIR_AUTOFREE(...) _VIR_AUTOFREE_OVERLOADER(__VA_ARGS__, _VIR_AUTOFREE_1, _VIR_AUTOFREE_0)(__VA_ARGS__) This would allow us to use any existing function directly with attribute cleanup. However, the function must take as argument pointer to the specified type so we would have to create a wrapper functions which would be used in the VIR_AUTOFREE macro, for example: void virStringListPtrFree(char ***stringsp) { virStringListFree(*stringsp); } VIR_AUTOFREE(char **, virStringListPtrFree) list = NULL; 2. External types with free function Libvirt uses a lot of external libraries where we use the types and relevant free functions. The issue with original design is that it was counting on the fact that we would have vir*Ptr typedefs, but that's not the case for external types. - We can modify the design to be closer to GLib design which would work even with external types and would be safe in case external free functions will not behave correctly. These are to modification to the original design: # define VIR_AUTOPTR_TYPE_NAME(type) ##typeAutoPtr # 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); \ *_ptr = NULL; \ } \ } # define VIR_AUTOPTR(type) \ __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) VIR_AUTOPTR_TYPE_NAME(type) The usage would not require our internal vir*Ptr types and would be easy to use with external types as well: VIR_DEFINE_AUTOPTR_FUNC(virBitmap, virBitmapFree); VIR_AUTOPTR(virBitmap) map = NULL; VIR_DEFINE_AUTOPTR_FUNC(LIBSSH2_CHANNEL, libssh2_channel_free); VIR_AUTOPTR(LIBSSH2_CHANNEL) channel = NULL; There was one suggestion to make another set of macros for the external types or just ignore them because there are only few places which uses external free functions. Pavel