Re: [PATCH 3/6] util: Add write_remainder() helper

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 22 Feb 2024 16:55:59 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> We have several places where we want to write(2) a buffer or buffers and we
> do (or should) handle sort write()s by retrying until everything is

After making sure that "handle sorting" doesn't exist (yet): is one
between "handle" and "sort" redundant, or is there another meaning to
this?

> successfully written.  Add a helper for this in util.c.
> 
> This version has some differences from the typical write_all() function.
> First, take an IO vector rather than a single buffer, because that will be
> useful for some of our cases.  Second, allow it to take an parameter to
> skip the first n bytes of the given buffers.  This will be usefl for some
> of the cases we want, and also falls out quite naturally from the
> implementation.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  util.c | 32 ++++++++++++++++++++++++++++++++
>  util.h |  1 +
>  2 files changed, 33 insertions(+)
> 
> diff --git a/util.c b/util.c
> index fb6a0430..a475f2b5 100644
> --- a/util.c
> +++ b/util.c
> @@ -19,6 +19,7 @@
>  #include <arpa/inet.h>
>  #include <net/ethernet.h>
>  #include <sys/epoll.h>
> +#include <sys/uio.h>
>  #include <fcntl.h>
>  #include <string.h>
>  #include <time.h>
> @@ -597,3 +598,34 @@ size_t iov_offset(const struct iovec *iov, size_t n, size_t *offset)
>  
>  	return i;
>  }
> +
> +/* write_remainder() - write the tail of an IO vector to an fd
> + * @fd:		File descriptor
> + * @iov:	IO vector
> + * @iovcnt:	Number of entries in @iov
> + * @skip:	Number of bytes of the vector to skip writing
> + *
> + * Return: 0 on success, -1 on error (with errno set)
> + *
> + * #syscalls write writev
> + */
> +int write_remainder(int fd, const struct iovec *iov, int iovcnt, size_t skip)
> +{
> +	int i;
> +
> +	while ((i = iov_offset(iov, iovcnt, &skip)) < iovcnt) {

With my proposal for 1/6 this becomes:

	while ((i = iov_entry_index(iov, iovcnt, skip, &offset)) < iovcnt) {

		if (offset)
			...

which I don't really find brilliant, but at least we don't do if (skip)
where 'skip' used to be something completely different.

> +		ssize_t rc;
> +
> +		if (skip)

Curly brackets here for consistency (undecided about readability to be
honest).

> +			rc = write(fd, (char *)iov[i].iov_base + skip,
> +				   iov[i].iov_len - skip);
> +		else
> +			rc = writev(fd, &iov[i], iovcnt - i);
> +
> +		if (rc < 0)
> +			return -1;
> +		skip += rc;
> +	}
> +
> +	return 0;
> +}
> diff --git a/util.h b/util.h
> index 62fad6fe..ee380f55 100644
> --- a/util.h
> +++ b/util.h
> @@ -230,6 +230,7 @@ int __daemon(int pidfile_fd, int devnull_fd);
>  int fls(unsigned long x);
>  int write_file(const char *path, const char *buf);
>  size_t iov_offset(const struct iovec *iov, size_t n, size_t *offset);
> +int write_remainder(int fd, const struct iovec *iov, int iovcnt, size_t skip);
>  
>  /**
>   * mod_sub() - Modular arithmetic subtraction

-- 
Stefano