Re: [PATCH 02/10] iov: Add iov_memcopy() to copy data between iovec arrays

[Stefano Brivio <sbrivio@redhat.com>]

Nits, all about names:

On Wed,  1 Apr 2026 21:18:18 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Add a helper to copy data from a source iovec array to a destination
> iovec array, each starting at an arbitrary byte offset, iterating
> through both arrays simultaneously and copying in chunks matching the
> smaller of the two current segments.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  iov.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  iov.h |  3 +++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/iov.c b/iov.c
> index 0188acdf5eba..83b683f3976a 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -197,6 +197,58 @@ void iov_memset(const struct iovec *iov, size_t iov_cnt, size_t offset, int c,
>  	}
>  }
>  
> +/**
> + * iov_memcopy() - Copy data between two iovec arrays

Wouldn't it be less surprising to call this iov_memcpy(), like memcpy()?

> + * @dst_iov:		Destination iovec array
> + * @dst_iov_cnt:	Number of elements in destination iovec array
> + * @dst_offs:		Destination offset
> + * @iov:		Source iovec array
> + * @iov_cnt:		Number of elements in source iovec array

I think @src_iov and @src_iov_cnt would make the whole function easier
to follow and look more symmetric.

> + * @offs:		Source offset

What about @dst_offset and @src_offset? "offs" as a short-form for
"offset" isn't really obvious (to me at least).

> + * @length:		Number of bytes to copy
> + *
> + * Return: total number of bytes copied
> + */
> +/* cppcheck-suppress unusedFunction */
> +size_t iov_memcopy(struct iovec *dst_iov, size_t dst_iov_cnt, size_t dst_offs,
> +		   const struct iovec *iov, size_t iov_cnt, size_t offs,
> +		   size_t length)
> +{
> +	unsigned int i, j;
> +	size_t total = 0;
> +
> +	i = iov_skip_bytes(iov, iov_cnt, offs, &offs);
> +	j = iov_skip_bytes(dst_iov, dst_iov_cnt, dst_offs, &dst_offs);
> +
> +	/* copying data */
> +	while (length && i < iov_cnt && j < dst_iov_cnt) {
> +		size_t n = MIN(dst_iov[j].iov_len - dst_offs,
> +			       iov[i].iov_len - offs);
> +
> +		if (n > length)
> +			n = length;
> +
> +		memcpy((char *)dst_iov[j].iov_base + dst_offs,
> +		       (const char *)iov[i].iov_base + offs, n);
> +
> +		dst_offs += n;
> +		offs += n;
> +		total += n;
> +		length -= n;
> +
> +		if (dst_offs == dst_iov[j].iov_len) {
> +			dst_offs = 0;
> +			j++;
> +		}
> +		if (offs == iov[i].iov_len) {
> +			offs = 0;
> +			i++;
> +		}
> +	}
> +
> +	return total;
> +}
> +
>  /**
>   * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
>   * @tail:	IO vector tail (modified)
> diff --git a/iov.h b/iov.h
> index d295d05b3bab..074266e127ef 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -32,6 +32,9 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt);
>  size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
>  void iov_memset(const struct iovec *iov, size_t iov_cnt, size_t offset, int c,
>  		size_t length);
> +size_t iov_memcopy(struct iovec *dst_iov, size_t dst_iov_cnt, size_t dst_offs,
> +		   const struct iovec *iov, size_t iov_cnt, size_t offs,
> +		   size_t length);
>  
>  /*
>   * DOC: Theory of Operation, struct iov_tail

-- 
Stefano