[libvirt] [GSoC] Code design for scalar and external types

[Pavel Hrdina]

Hi,

It took me longer to sit down and write this mail but here it goes.

There was a lot of ideas about the macro design and it's easy to
get lost in all the designs.

So we agreed on this form:


# define VIR_AUTOPTR_FUNC_NAME(type) virAutoPtr##type
# define VIR_AUTOCLEAR_FUNC_NAME(type) virAutoClear##type

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

# define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \
    static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \
    { \
        (func)(_ptr); \
    }

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

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

# define VIR_AUTOCLEAR(type) \
    __attribute__((cleanup(VIR_AUTOCLEAR_FUNC_NAME(type)))) type


but it turned out the it's not sufficient for several reasons:

    - there is no simple way ho to free list of strings (char **)
    - this will not work for external types with their own free function


In order to solve these two issues there were some ideas.


1. How to handle list of strings

We have virStringListFree() function in order to free list of strings,
the question is how to use it with these macros:

    - Create a new typedef for list of strings and use VIR_AUTOPTR:

typedef char **virStringList;

VIR_DEFINE_AUTOPTR_FUNC(virStringList, virStringListFree);

VIR_AUTOPTR(virStringList) list = NULL;


      The benefit of this solution is that it works with current design
      and we only need to create single typedef.

    - Overload VIR_AUTOFREE macro by making it variadic

# define _VIR_AUTOFREE_0(type) __attribute__((cleanup(virFree))) type
# define _VIR_AUTOFREE_1(type, func) __attribute__((cleanup(func))) type

# define _VIR_AUTOFREE_OVERLOADER(_1, _2, NAME, ...) NAME
# define VIR_AUTOFREE(...) _VIR_AUTOFREE_OVERLOADER(__VA_ARGS__, _VIR_AUTOFREE_1, _VIR_AUTOFREE_0)(__VA_ARGS__)

      This would allow us to use any existing function directly with
      attribute cleanup.  However, the function must take as argument
      pointer to the specified type so we would have to create a wrapper
      functions which would be used in the VIR_AUTOFREE macro, for
      example:

void virStringListPtrFree(char ***stringsp)
{
    virStringListFree(*stringsp);
}

VIR_AUTOFREE(char **, virStringListPtrFree) list = NULL;


2. External types with free function

Libvirt uses a lot of external libraries where we use the types and
relevant free functions.  The issue with original design is that it was
counting on the fact that we would have vir*Ptr typedefs, but that's not
the case for external types.

    - We can modify the design to be closer to GLib design which would
      work even with external types and would be safe in case external
      free functions will not behave correctly.  These are to
      modification to the original design:

# define VIR_AUTOPTR_TYPE_NAME(type) ##typeAutoPtr

# 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; \
        } \
    }

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

      The usage would not require our internal vir*Ptr types and would
      be easy to use with external types as well:

VIR_DEFINE_AUTOPTR_FUNC(virBitmap, virBitmapFree);

VIR_AUTOPTR(virBitmap) map = NULL;

VIR_DEFINE_AUTOPTR_FUNC(LIBSSH2_CHANNEL, libssh2_channel_free);

VIR_AUTOPTR(LIBSSH2_CHANNEL) channel = NULL;

      There was one suggestion to make another set of macros for the
      external types or just ignore them because there are only few
      places which uses external free functions.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180618/42235c65/attachment-0001.sig>