Re: [libvirt] [PATCH 2/4] util: alloc: Introduce macro for self-freeing NULL-terminated lists
On Fri, Feb 22, 2019 at 05:04:38PM +0100, Peter Krempa wrote:
VIR_AUTOLISTPTR allows you to define a pointer to a NULL-terminated
list
of elements which will be auto-freed when destroying the scope.
This is done so that we can avoid using VIR_AUTOPTR in those cases as it
worked only for virStringList-related stuff.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/util/viralloc.h | 61 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 50a07d4fa3..983a6e83d1 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -631,6 +631,54 @@ void virAllocTestHook(void (*func)(int, void*), void *data);
(func)(_ptr); \
}
+
+# define VIR_AUTOLISTPTR_FUNC_NAME(type) type##AutoListPtrFree
+
+/**
+ * VIR_DEFINE_AUTOLISTPTR_FUNC:
+ * @type: type of the element of the list
+ * @func: freeing function for a single element of the list
+ *
+ * This macro defines the freeing function used by VIR_AUTOLISTPTR for @type.
+ * @func must free one of the elements of @type.
+ *
+ * Note that the function is not inline due to size and thus
+ * VIR_DECLARE_AUTOLISTPTR_FUNC needs to be used in the appropriate header file.
+ *
+ * VIR_DEFINE_AUTOLISTPTR_FUNC is mutually exclusive with
+ * VIR_DEFINE_AUTOLISTPTR_FUNC_DIRECT.
+ */
+# define VIR_DECLARE_AUTOLISTPTR_FUNC(type) \
+ void VIR_AUTOLISTPTR_FUNC_NAME(type)(type **_ptr);
+# define VIR_DEFINE_AUTOLISTPTR_FUNC(type, func) \
+ void VIR_AUTOLISTPTR_FUNC_NAME(type)(type **_ptr) \
+ { \
+ type *n;\
+ for (n = *_ptr; n && *n; n++) \
+ (func)(n); \
+ VIR_FREE(*_ptr);\
+ }
So, I believe that ^this should be unnecessary and it should always be handled
by the function. I don't have a good argument yet, I need to think about this
some more, but at the time being, I think that if we have a type from which we
create a NULL-terminated list, we should have dedicated type for that too and a
destructor as well, ergo VIR_AUTOPTR suffices. I appreciate and understand the
effort, solely because it was virString driving it which is confusing on its
own because we can't make AUTOPTR work properly because virString introduces an
additional pointer and everything would go haywire if we had virStringList
type. Maybe it would be worth saying that virString is indeed special and
therefore needs to be handled on its own with its own macro. That way, the whole
AUTOLIST wouldn't be needed, at least not in the current form.
I can be convinced otherwise with good arguments though.
+
+/**
+ * VIR_DEFINE_AUTOLISTPTR_FUNC_DIRECT:
+ * @type: type of the element of the list
+ * @func: freeing function for the whole NULL-terminated list of @type
+ *
+ * This macro defines the freeing function used by VIR_AUTOLISTPTR for @type.
+ * @func must free a NULL terminated dense list of @type and also the list
+ * itself. This is a convenience option if @type has already such function.
+ *
+ * VIR_DEFINE_AUTOLISTPTR_FUNC_DIRECT is mutually exclusive with
+ * VIR_DEFINE_AUTOLISTPTR_FUNC.
+ */
+# define VIR_DEFINE_AUTOLISTPTR_FUNC_DIRECT(type, func) \
+ static inline void VIR_AUTOLISTPTR_FUNC_NAME(type)(type **_ptr) \
+ { \
+ if (*_ptr) \
+ (func)(*_ptr); \
+ }
So the only difference between ^this and VIR_AUTOPTR is the description.
Erik