[libvirt] [PATCH v1 22/32] util: netlink: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC

[Erik Skultety]

On Sat, Jul 28, 2018 at 11:31:37PM +0530, Sukrit Bhatnagar wrote:
> Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in
> src/util/viralloc.h, define a new wrapper around an existing
> cleanup function which will be called when a variable declared
> with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant
> viralloc.h include, since that has moved from the source module into
> the header.
>
> This commit also adds a typedef for virNetlinkHandlePtr for use with
> the cleanup macros.
>
> When a variable of type virNetlinkHandlePtr is declared using
> VIR_AUTOPTR, the function virNetlinkFree will be run automatically
> on it when it goes out of scope.
>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr at gmail.com>
> ---
>  src/util/virnetlink.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
> index fa1ba3e..8d28387 100644
> --- a/src/util/virnetlink.c
> +++ b/src/util/virnetlink.c
> @@ -72,6 +72,10 @@ typedef struct nl_handle virNetlinkHandle;
>  typedef struct nl_sock virNetlinkHandle;
>  # endif
>
> +typedef virNetlinkHandle *virNetlinkHandlePtr;

Yes, ^this somewhat brings consistency, but it's nothing more than a syntactic
sugar we technically don't need for the attribute cleanup functionality unless
we'll have something like a list of virNetlinkHandles. Although harmless and
correct, I think that it would make much more sense if we simply let the
contributor who's going to add a list of virNetlinkHandles as a NULL-terminated
list in the future to create this typedef as well, it'll make more sense in the
context of such a patch because we'll actually need it, because we wouldn't
have the necessary type for the corresponding *Free method.

> +
> +VIR_DEFINE_AUTOPTR_FUNC(virNetlinkHandle, virNetlinkFree)

To ^this:
Reviewed-by: Erik Skultety <eskultet at redhat.com>

> +
>  typedef struct _virNetlinkEventSrvPrivate virNetlinkEventSrvPrivate;
>  typedef virNetlinkEventSrvPrivate *virNetlinkEventSrvPrivatePtr;
>  struct _virNetlinkEventSrvPrivate {
> @@ -79,7 +83,7 @@ struct _virNetlinkEventSrvPrivate {
>      virMutex lock;
>      int eventwatch;
>      int netlinkfd;
> -    virNetlinkHandle *netlinknh;
> +    virNetlinkHandlePtr netlinknh;

^these renames would only add to the noise in the history, we don't benefit
from this rename at all, so all of ^these changes should be dropped
unconditionally...

Erik