Re: [libvirt] [PATCH for v4.6.0] cgroups: Don't leave stale pointers around after virCgroupFree

On Mon, Jul 30, 2018 at 09:58:41AM +0100, Daniel P. Berrangé wrote: I agree that this change was not correct and we should revert it, the ultimate goal should be to have all the *Free() functions to take double pointer and set the variable to NULL as well. We already had this discussion, there are two possibilities: 1) as the end goal simple wrapper function defined by MACRO: # define VIR_DEFINE_AUTOPTR_FUNC(type, func) \ static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \ { \ (func)(_ptr); \ } \ It's a static inline so it will disappear during compilation. As it may look like unnecessary code there is one benefit while using VIR_AUTOPTR() 2) the second option is to have a macro that would replace __attribute__(cleanup()), for example: VIR_CLEANUP(func) __attribute__(cleanup(func)) If you compare the usage: VIR_AUTOPTR(virCgroup) group = NULL; VIR_CLEANUP(virCgroupFree) virCgroupPtr group = NULL; I personally prefer the first option, it's cleaner and the line is shorter and it's perfectly readable even though some logic is hidden. The second usage has a benefit that the logic is not hidden and you can use any function and you don't have to define anything but the line can get really long and ugly as we have some really long names for Free functions and types. For example: VIR_AUTOPTR(virDomainGraphicsDef) def = NULL; VIR_CLEANUP(virDomainGraphicsDefFree) virDomainGraphicsDefPtr def = NULL; And that's not even the longest free function. Since the wrapper defined by VIR_DEFINE_AUTOPTR_FUNC() will be expanded during compilation I prefer that option. It would essentially work the same way is VIR_CLEANUP but with the benefit that the declaration line is short, nice and clean. Pavel