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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Andrew Cooper writes ("[PATCH] tools/libxc: Implement writev_exact() in the 
same style as write_exact()"):
> 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.
...
> +int writev_exact(int fd, 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;

If writev does return 0, you at the very least need to set errno
before changing the return value to -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;
> +
> +        if ( len < 0 ) /* Partial write of iov[iov_idx - 1]. */
> +        {
> +            iov_idx--;

This logic is rather wtf!  Isn't there a way of expressing it that
doesn't involve len becoming negative and unwinding iov_idx ?

> +int writev_exact(int fd, struct iovec *iov, int iovcnt);
> +/* Note - writev_exact() might modify iov.  Whether it does so in practice
> + * depends on whether your system implementation of writev() returns from a
> + * partial write in the middle of an iov element. */

The second sentence should be removed.  No-one is allowed to assume
that writev doesn't do so.  Also, you should mention that your
writev_exact lacks the atomicity guarantee of proper writev.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel