Re: [PATCH v2] tools/libxc: Implement writev_exact() in the same style as write_exact()

[Hongyang Yang <yanghy@cn.fujitsu.com>]

On 07/07/2014 06:18 PM, Andrew Cooper wrote:
> This implementation of writev_exact() will cope with an iovcnt greater than
> IOV_MAX because glibc will actually let this work anyway, and it is very
> useful not to have to work about this in the caller of writev_exact().  The
> caller is still required to ensure that the sum of iov_len's doesn't overflow
> a ssize_t.
>
> Promote the MAX() and MIN() macro definitions from xg_save_restore.h to
> xc_private.h
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>

Reviewed-by: Yang Hongyang <yanghy@cn.fujitsu.com>

>
> ---
>
> v2: Remove adjustment for partial writes of a specific iov[] entry.
> ---
>   tools/libxc/xc_private.c      |   31 +++++++++++++++++++++++++++++++
>   tools/libxc/xc_private.h      |    9 +++++++++
>   tools/libxc/xg_save_restore.h |    6 ------
>   3 files changed, 40 insertions(+), 6 deletions(-)
>
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index a3da614..d610984 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -854,6 +854,37 @@ int write_exact(int fd, const void *data, size_t size)
>       return 0;
>   }
>
> +int writev_exact(int fd, const struct iovec *iov, int iovcnt)
> +{
> +    int iov_idx = 0;
> +    ssize_t len;
> +
> +    while ( iov_idx < iovcnt )
> +    {
> +        /* Skip over iov[] enties with 0 length. */
> +        while ( iov[iov_idx].iov_len == 0 )
> +            if ( ++iov_idx == iovcnt )
> +                return 0;
> +
> +        len = writev(fd, &iov[iov_idx], MIN(iovcnt - iov_idx, IOV_MAX));
> +
> +        if ( (len == -1) && (errno == EINTR) )
> +            continue;
> +        if ( len <= 0 )
> +            return -1;
> +
> +        /* Check iov[] to see whether we had a partial or complete write. */
> +        while ( len > 0 && (iov_idx < iovcnt) )
> +            len -= iov[iov_idx++].iov_len;
> +
> +        /* writev() guarentees atomicity of individual iov[] elements.  Sanity
> +         * check that the returned len did lie on an iov[] element boundary. */
> +        assert(len == 0);
> +    }
> +
> +    return 0;
> +}
> +
>   int xc_ffs8(uint8_t x)
>   {
>       int i;
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index 6cc0f2b..198b8a0 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -28,6 +28,7 @@
>   #include <sys/stat.h>
>   #include <stdlib.h>
>   #include <sys/ioctl.h>
> +#include <sys/uio.h>
>
>   #include "xenctrl.h"
>   #include "xenctrlosdep.h"
> @@ -343,6 +344,7 @@ int xc_flush_mmu_updates(xc_interface *xch, struct xc_mmu *mmu);
>   /* Return 0 on success; -1 on error setting errno. */
>   int read_exact(int fd, void *data, size_t size); /* EOF => -1, errno=0 */
>   int write_exact(int fd, const void *data, size_t size);
> +int writev_exact(int fd, const struct iovec *iov, int iovcnt);
>
>   int xc_ffs8(uint8_t x);
>   int xc_ffs16(uint16_t x);
> @@ -367,4 +369,11 @@ int xc_mem_event_memop(xc_interface *xch, domid_t domain_id,
>   void *xc_mem_event_enable(xc_interface *xch, domid_t domain_id, int param,
>                             uint32_t *port);
>
> +#ifndef MAX
> +#define MAX(_a, _b) ((_a) >= (_b) ? (_a) : (_b))
> +#endif
> +#ifndef MIN
> +#define MIN(_a, _b) ((_a) <= (_b) ? (_a) : (_b))
> +#endif
> +
>   #endif /* __XC_PRIVATE_H__ */
> diff --git a/tools/libxc/xg_save_restore.h b/tools/libxc/xg_save_restore.h
> index aa93c13..34019fa 100644
> --- a/tools/libxc/xg_save_restore.h
> +++ b/tools/libxc/xg_save_restore.h
> @@ -393,9 +393,3 @@ static inline int get_platform_info(xc_interface *xch, uint32_t dom,
>           memset(&(_p)->x32._f[0], (_v), sizeof((_p)->x32._f));      \
>   } while (0)
>
> -#ifndef MAX
> -#define MAX(_a, _b) ((_a) >= (_b) ? (_a) : (_b))
> -#endif
> -#ifndef MIN
> -#define MIN(_a, _b) ((_a) <= (_b) ? (_a) : (_b))
> -#endif
>

-- 
Thanks,
Yang.