[libvirt] [PATCH 1/6] util: file: introduce VIR_AUTOCLOSE macro to close fd of the file automatically

[Michal Privoznik]

On 09/10/2018 05:47 AM, Shi Lei wrote:
> By making use of GNU C's cleanup attribute handled by the VIR_AUTOCLOSE macro,
> many of the VIR_FORCE_CLOSE calls can be dropped, which in turn leads to
> getting rid of many of our cleanup sections.
>     
> Signed-off-by: Shi Lei <shi_lei at massclouds.com>
> ---
>  src/util/virfile.h | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/virfile.h b/src/util/virfile.h
> index b30a1d3..70e7203 100644
> --- a/src/util/virfile.h
> +++ b/src/util/virfile.h
> @@ -54,6 +54,11 @@ int virFileClose(int *fdptr, virFileCloseFlags flags)
>  int virFileFclose(FILE **file, bool preserve_errno) ATTRIBUTE_RETURN_CHECK;
>  FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
>  
> +static inline void virForceCloseHelper(int *_fd)

No need for this argument to have underscore in its name.

> +{
> +    ignore_value(virFileClose(_fd, VIR_FILE_CLOSE_PRESERVE_ERRNO));
> +}
> +
>  /* For use on normal paths; caller must check return value,
>     and failure sets errno per close. */
>  # define VIR_CLOSE(FD) virFileClose(&(FD), 0)
> @@ -64,8 +69,7 @@ FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
>  
>  /* For use on cleanup paths; errno is unaffected by close,
>     and no return value to worry about. */
> -# define VIR_FORCE_CLOSE(FD) \
> -    ignore_value(virFileClose(&(FD), VIR_FILE_CLOSE_PRESERVE_ERRNO))
> +# define VIR_FORCE_CLOSE(FD) virForceCloseHelper(&(FD))
>  # define VIR_FORCE_FCLOSE(FILE) ignore_value(virFileFclose(&(FILE), true))
>  
>  /* Similar VIR_FORCE_CLOSE() but ignores EBADF errors since they are expected
> @@ -80,6 +84,18 @@ FILE *virFileFdopen(int *fdptr, const char *mode) ATTRIBUTE_RETURN_CHECK;
>                   VIR_FILE_CLOSE_PRESERVE_ERRNO | \
>                   VIR_FILE_CLOSE_DONT_LOG))
>  
> +/**
> + * VIR_AUTOCLOSE:
> + * @fd: fd of the file to be closed automatically
> + *
> + * Macro to automatically force close the fd by calling virForceCloseHelper
> + * when the fd goes out of scope. It's used to eliminate VIR_FORCE_CLOSE
> + * in cleanup sections.
> + */
> +# define VIR_AUTOCLOSE(fd) \
> +    __attribute__((cleanup(virForceCloseHelper))) int fd = -1

While this may helps us to initialize variables correctly, I think we
should do that explicitly. Not only it follows what VIR_AUTOFREE is
doing, it also is more visible when used. For instance, in 2/6 when the
macro is used for the first time, it's not visible what is @fd
initialized to.

> +
> +
>  /* Opaque type for managing a wrapper around a fd.  */
>  struct _virFileWrapperFd;
>  
> 

Otherwise the rest of the series looks good.

Michal