[libvirt] [PATCH 2/4] util: alloc: Introduce macro for self-freeing NULL-terminated lists

Peter Krempa posted 4 patches 6 years, 11 months ago
[libvirt] [PATCH 2/4] util: alloc: Introduce macro for self-freeing NULL-terminated lists
Posted by Peter Krempa 6 years, 11 months ago
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@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);\
+    }
+
+/**
+ * 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); \
+    }
+
+
 /**
  * VIR_AUTOFREE:
  * @type: type of the variable to be freed automatically
@@ -665,6 +713,19 @@ void virAllocTestHook(void (*func)(int, void*), void *data);
 # define VIR_AUTOCLEAN(type) \
     __attribute__((cleanup(VIR_AUTOCLEAN_FUNC_NAME(type)))) type

+/**
+ * VIR_AUTOLISTPTR:
+ * @type: type of the members of the self-freeing NULL-terminated list to be defined
+ *
+ * This macro defines a pointer to a NULL-terminated list of @type members. The
+ * list is automatically freed when it goes out of scope including elements
+ * themselves.
+ *
+ * The freeing function is registered by VIR_DEFINE_AUTOLISTPTR_FUNC or
+ * VIR_DEFINE_AUTOLISTPTR_FUNC_DIRECT macro for the given type.
+ */
+# define VIR_AUTOLISTPTR(type) \
+    __attribute__((cleanup(VIR_AUTOLISTPTR_FUNC_NAME(type)))) type *

 /**
  * VIR_AUTOUNREF:
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] util: alloc: Introduce macro for self-freeing NULL-terminated lists
Posted by Erik Skultety 6 years, 11 months ago
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@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list