On Thu, Jul 12, 2018 at 11:44:16PM +0530, Sukrit Bhatnagar wrote:
> On Thu, 12 Jul 2018 at 22:09, Erik Skultety <eskultet@xxxxxxxxxx> 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 <skrtbhtngr@xxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
