Re: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality

2018-07-13 Thread Erik Skultety
On Thu, Jul 12, 2018 at 11:44:16PM +0530, Sukrit Bhatnagar wrote:
> On Thu, 12 Jul 2018 at 22:09, Erik Skultety  wrote:
> >
> > On Sat, Jun 30, 2018 at 02:30:05PM +0530, Sukrit Bhatnagar wrote:
> > > New macros are introduced which help in adding GNU C's cleanup
> > > attribute to variable declarations. Variables declared with these
> > > macros will have their allocated memory freed automatically when
> > > they go out of scope.
> > >
> > > Signed-off-by: Sukrit Bhatnagar 
> > > ---
> > >  src/util/viralloc.h | 44 
> > >  1 file changed, 44 insertions(+)
> > >
> > > diff --git a/src/util/viralloc.h b/src/util/viralloc.h
> > > index 69d0f90..5c1d0d5 100644
> > > --- a/src/util/viralloc.h
> > > +++ b/src/util/viralloc.h
> > > @@ -596,4 +596,48 @@ void virAllocTestInit(void);
> > >  int virAllocTestCount(void);
> > >  void virAllocTestOOM(int n, int m);
> > >  void virAllocTestHook(void (*func)(int, void*), void *data);
> > > +
> > > +# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
> > > +# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
> > > +
> > > +/**
> > > + * VIR_DEFINE_AUTOPTR_FUNC:
> > > + * @type: type of the variable to be freed automatically
> > > + * @func: cleanup function to be automatically called
> > > + *
> > > + * This macro defines a function for automatic freeing of
> > > + * resources allocated to a variable of type @type. This newly
> > > + * defined function works as a necessary wrapper around @func.
> > > + */
> > > +# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> > > +typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
> >
> > So, it's not visible at first glance how ^this typedef is used...
> >
> > > +static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
> > > +{ \
> > > +if (*_ptr) \
> > > +(func)(*_ptr); \
> > > +*_ptr = NULL; \
> > > +} \
> >
> > ...therefore I'd write it explicitly as
> >
> > VIR_AUTOPTR_FUNC_NAME(type)(VIR_AUTOPTR_TYPE_NAME(type) *_ptr)
> >
> > > +
> > > +# define VIR_AUTOPTR(type) \
> > > +__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type 
> > > VIR_AUTOPTR_TYPE_NAME(type)
> > > +
> >
> > Also, since we're going to use it like this:
> > VIR_AUTOPTR(virDomainDef) foo
> >
> > instead of this:
> > VIR_AUTOPTR(virDomainDefPtr) foo
> >
> > why do we need VIR_AUTOPTR_TYPE_NAME anyway, since you could just use
> > "type *" in the VIR_AUTOPTR macro definition and that should work for 
> > external
> > types as well.
>
> I agree. We don't really need it. Here is how the code will look after
> the changes
> you suggested:
>
> # define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
>
> # define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
> { \
> if (*_ptr) \
> (func)(*_ptr); \
> *_ptr = NULL; \
> } \
>
> # define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
>
> # define VIR_AUTOPTR(type) \
> __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type type *
>
>
> Shall I proceed to send the first series of patches?

Yep, please do, since there weren't any major flaws code-wise, I think it
should be very straightforward.

Erik

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


Re: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality

2018-07-12 Thread Sukrit Bhatnagar
On Thu, 12 Jul 2018 at 22:09, Erik Skultety  wrote:
>
> On Sat, Jun 30, 2018 at 02:30:05PM +0530, Sukrit Bhatnagar wrote:
> > New macros are introduced which help in adding GNU C's cleanup
> > attribute to variable declarations. Variables declared with these
> > macros will have their allocated memory freed automatically when
> > they go out of scope.
> >
> > Signed-off-by: Sukrit Bhatnagar 
> > ---
> >  src/util/viralloc.h | 44 
> >  1 file changed, 44 insertions(+)
> >
> > diff --git a/src/util/viralloc.h b/src/util/viralloc.h
> > index 69d0f90..5c1d0d5 100644
> > --- a/src/util/viralloc.h
> > +++ b/src/util/viralloc.h
> > @@ -596,4 +596,48 @@ void virAllocTestInit(void);
> >  int virAllocTestCount(void);
> >  void virAllocTestOOM(int n, int m);
> >  void virAllocTestHook(void (*func)(int, void*), void *data);
> > +
> > +# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
> > +# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
> > +
> > +/**
> > + * VIR_DEFINE_AUTOPTR_FUNC:
> > + * @type: type of the variable to be freed automatically
> > + * @func: cleanup function to be automatically called
> > + *
> > + * This macro defines a function for automatic freeing of
> > + * resources allocated to a variable of type @type. This newly
> > + * defined function works as a necessary wrapper around @func.
> > + */
> > +# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> > +typedef type *VIR_AUTOPTR_TYPE_NAME(type); \
>
> So, it's not visible at first glance how ^this typedef is used...
>
> > +static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
> > +{ \
> > +if (*_ptr) \
> > +(func)(*_ptr); \
> > +*_ptr = NULL; \
> > +} \
>
> ...therefore I'd write it explicitly as
>
> VIR_AUTOPTR_FUNC_NAME(type)(VIR_AUTOPTR_TYPE_NAME(type) *_ptr)
>
> > +
> > +# define VIR_AUTOPTR(type) \
> > +__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type 
> > VIR_AUTOPTR_TYPE_NAME(type)
> > +
>
> Also, since we're going to use it like this:
> VIR_AUTOPTR(virDomainDef) foo
>
> instead of this:
> VIR_AUTOPTR(virDomainDefPtr) foo
>
> why do we need VIR_AUTOPTR_TYPE_NAME anyway, since you could just use
> "type *" in the VIR_AUTOPTR macro definition and that should work for external
> types as well.

I agree. We don't really need it. Here is how the code will look after
the changes
you suggested:

# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree

# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
{ \
if (*_ptr) \
(func)(*_ptr); \
*_ptr = NULL; \
} \

# define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type

# define VIR_AUTOPTR(type) \
__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type type *


Shall I proceed to send the first series of patches?

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


Re: [libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality

2018-07-12 Thread Erik Skultety
On Sat, Jun 30, 2018 at 02:30:05PM +0530, Sukrit Bhatnagar wrote:
> New macros are introduced which help in adding GNU C's cleanup
> attribute to variable declarations. Variables declared with these
> macros will have their allocated memory freed automatically when
> they go out of scope.
>
> Signed-off-by: Sukrit Bhatnagar 
> ---
>  src/util/viralloc.h | 44 
>  1 file changed, 44 insertions(+)
>
> diff --git a/src/util/viralloc.h b/src/util/viralloc.h
> index 69d0f90..5c1d0d5 100644
> --- a/src/util/viralloc.h
> +++ b/src/util/viralloc.h
> @@ -596,4 +596,48 @@ void virAllocTestInit(void);
>  int virAllocTestCount(void);
>  void virAllocTestOOM(int n, int m);
>  void virAllocTestHook(void (*func)(int, void*), void *data);
> +
> +# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
> +# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
> +
> +/**
> + * VIR_DEFINE_AUTOPTR_FUNC:
> + * @type: type of the variable to be freed automatically
> + * @func: cleanup function to be automatically called
> + *
> + * This macro defines a function for automatic freeing of
> + * resources allocated to a variable of type @type. This newly
> + * defined function works as a necessary wrapper around @func.
> + */
> +# define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> +typedef type *VIR_AUTOPTR_TYPE_NAME(type); \

So, it's not visible at first glance how ^this typedef is used...

> +static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \
> +{ \
> +if (*_ptr) \
> +(func)(*_ptr); \
> +*_ptr = NULL; \
> +} \

...therefore I'd write it explicitly as

VIR_AUTOPTR_FUNC_NAME(type)(VIR_AUTOPTR_TYPE_NAME(type) *_ptr)

> +
> +# define VIR_AUTOPTR(type) \
> +__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type 
> VIR_AUTOPTR_TYPE_NAME(type)
> +

Also, since we're going to use it like this:
VIR_AUTOPTR(virDomainDef) foo

instead of this:
VIR_AUTOPTR(virDomainDefPtr) foo

why do we need VIR_AUTOPTR_TYPE_NAME anyway, since you could just use
"type *" in the VIR_AUTOPTR macro definition and that should work for external
types as well.

Erik

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


[libvirt] [PATCH v3 01/35] util: alloc: add macros for implementing automatic cleanup functionality

2018-07-01 Thread Sukrit Bhatnagar
New macros are introduced which help in adding GNU C's cleanup
attribute to variable declarations. Variables declared with these
macros will have their allocated memory freed automatically when
they go out of scope.

Signed-off-by: Sukrit Bhatnagar 
---
 src/util/viralloc.h | 44 
 1 file changed, 44 insertions(+)

diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 69d0f90..5c1d0d5 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -596,4 +596,48 @@ void virAllocTestInit(void);
 int virAllocTestCount(void);
 void virAllocTestOOM(int n, int m);
 void virAllocTestHook(void (*func)(int, void*), void *data);
+
+# define VIR_AUTOPTR_TYPE_NAME(type) type##AutoPtr
+# define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree
+
+/**
+ * VIR_DEFINE_AUTOPTR_FUNC:
+ * @type: type of the variable to be freed automatically
+ * @func: cleanup function to be automatically called
+ *
+ * This macro defines a function for automatic freeing of
+ * resources allocated to a variable of type @type. This newly
+ * defined function works as a necessary wrapper around @func.
+ */
+# 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; \
+} \
+
+/**
+ * VIR_AUTOFREE:
+ * @type: type of the variable to be freed automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable declared with it by calling virFree
+ * when the variable goes out of scope.
+ */
+# define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
+
+/**
+ * VIR_AUTOPTR:
+ * @type: type of the variable to be freed automatically
+ *
+ * Macro to automatically free the memory allocated to
+ * the variable declared with it by calling the function
+ * defined by VIR_DEFINE_AUTOPTR_FUNC when the variable
+ * goes out of scope.
+ */
+# define VIR_AUTOPTR(type) \
+__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type 
VIR_AUTOPTR_TYPE_NAME(type)
+
 #endif /* __VIR_MEMORY_H_ */
-- 
1.8.3.1

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