[PATCH] iov: Fix coding style of basic (non-IOV_TAIL) parts

[PATCH] iov: Fix coding style of basic (non-IOV_TAIL) parts

[Stefano Brivio]

I happened to touch these functions for a change that turned out to be
unnecessary, but coding style and documentation could still benefit
from some rephrasing and fixes.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 iov.c | 68 +++++++++++++++++++++++++----------------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

diff --git a/iov.c b/iov.c
index 8c74a59..d31464d 100644
--- a/iov.c
+++ b/iov.c
@@ -20,24 +20,21 @@
  * Contributions after 2012-01-13 are licensed under the terms of the
  * GNU GPL, version 2 or (at your option) any later version.
  */
+
 #include <sys/socket.h>
 
 #include "util.h"
 #include "iov.h"
 
-
 /**
- * iov_skip_bytes() - Skip leading bytes of an IO vector
- * @iov:	IO vector
+ * iov_skip_bytes() - Find index and offset in iovec array given byte offset
+ * @iov:	iovec array
  * @n:		Number of entries in @iov
- * @skip:	Number of leading bytes of @iov to skip
- * @offset:	Offset of first unskipped byte in its @iov entry
+ * @skip:	Byte offset: leading bytes of @iov to skip
+ * @offset:	Offset within matching @iov entry, set on return, can be NULL
  *
- * Return: index I of individual struct iovec which contains the byte at @skip
- *         bytes into the vector (as though all its buffers were contiguous).
- *         If @offset is non-NULL, update it to the offset of that byte within
- *         @iov[I] (guaranteed to be less than @iov[I].iov_len) If the whole
- *         vector has <= @skip bytes, return @n.
+ * Return: index of iovec array containing the @skip byte counted as if buffers
+ *	   were contiguous. If iovec has less than @skip bytes, return @n.
  */
 size_t iov_skip_bytes(const struct iovec *iov, size_t n,
 		      size_t skip, size_t *offset)
@@ -57,17 +54,14 @@ size_t iov_skip_bytes(const struct iovec *iov, size_t n,
 }
 
 /**
- * iov_from_buf() - Copy data from a buffer to an I/O vector (struct iovec)
- *                  efficiently.
- *
- * @iov:       Pointer to the array of struct iovec describing the
- *             scatter/gather I/O vector.
- * @iov_cnt:   Number of elements in the iov array.
- * @offset:    Byte offset in the iov array where copying should start.
- * @buf:       Pointer to the source buffer containing the data to copy.
- * @bytes:     Total number of bytes to copy from buf to iov.
+ * iov_from_buf() - Copy from flat buffer to iovec array
+ * @iov:	Destination iovec array
+ * @iov_cnt:	Number of elements in the iovec array
+ * @offset:	Destination offset in iovec array
+ * @buf:	Source buffer
+ * @bytes:	Bytes to copy
  *
- * Return: the number of bytes successfully copied.
+ * Return: number of bytes copied
  */
 size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
 		    size_t offset, const void *buf, size_t bytes)
@@ -78,12 +72,12 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
 	if (__builtin_constant_p(bytes) && iov_cnt &&
 		offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
 		memcpy((char *)iov[0].iov_base + offset, buf, bytes);
+
 		return bytes;
 	}
 
 	i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
 
-	/* copying data */
 	for (copied = 0; copied < bytes && i < iov_cnt; i++) {
 		size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
 
@@ -97,17 +91,14 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
 }
 
 /**
- * iov_to_buf() - Copy data from a scatter/gather I/O vector (struct iovec) to
- *		  a buffer efficiently.
- *
- * @iov:       Pointer to the array of struct iovec describing the scatter/gather
- *             I/O vector.
- * @iov_cnt:   Number of elements in the iov array.
- * @offset:    Offset within the first element of iov from where copying should start.
- * @buf:       Pointer to the destination buffer where data will be copied.
- * @bytes:     Total number of bytes to copy from iov to buf.
+ * iov_to_buf() - Copy from iovec to flat buffer
+ * @iov:	Source iovec array
+ * @iov_cnt:	Number of elements in iovec array
+ * @offset:	Source offset in @iov, counted as if buffers were contiguous
+ * @buf:	Destination buffer
+ * @bytes:	Bytes to copy
  *
- * Return: the number of bytes successfully copied.
+ * Return: number of bytes copied
  */
 size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
 		  size_t offset, void *buf, size_t bytes)
@@ -118,15 +109,17 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
 	if (__builtin_constant_p(bytes) && iov_cnt &&
 		offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
 		memcpy(buf, (char *)iov[0].iov_base + offset, bytes);
+
 		return bytes;
 	}
 
 	i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
 
-	/* copying data */
 	for (copied = 0; copied < bytes && i < iov_cnt; i++) {
 		size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
+
 		ASSERT(iov[i].iov_base);
+
 		memcpy((char *)buf + copied, (char *)iov[i].iov_base + offset,
 		       len);
 		copied += len;
@@ -137,14 +130,11 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
 }
 
 /**
- * iov_size() - Calculate the total size of a scatter/gather I/O vector
- *              (struct iovec).
+ * iov_size() - Calculate total data size of iovec
+ * @iov:	Source iovec array
+ * @iov_cnt:	Number of elements in iovec array
  *
- * @iov:       Pointer to the array of struct iovec describing the
- *             scatter/gather I/O vector.
- * @iov_cnt:   Number of elements in the iov array.
- *
- * Return: the total size in bytes.
+ * Return: total size in bytes
  */
 size_t iov_size(const struct iovec *iov, size_t iov_cnt)
 {
-- 
2.43.0

Re: [PATCH] iov: Fix coding style of basic (non-IOV_TAIL) parts

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6448 bytes --]

On Fri, Dec 05, 2025 at 01:51:11AM +0100, Stefano Brivio wrote:
> I happened to touch these functions for a change that turned out to be
> unnecessary, but coding style and documentation could still benefit
> from some rephrasing and fixes.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

One nit below..

> ---
>  iov.c | 68 +++++++++++++++++++++++++----------------------------------
>  1 file changed, 29 insertions(+), 39 deletions(-)
> 
> diff --git a/iov.c b/iov.c
> index 8c74a59..d31464d 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -20,24 +20,21 @@
>   * Contributions after 2012-01-13 are licensed under the terms of the
>   * GNU GPL, version 2 or (at your option) any later version.
>   */
> +
>  #include <sys/socket.h>
>  
>  #include "util.h"
>  #include "iov.h"
>  
> -
>  /**
> - * iov_skip_bytes() - Skip leading bytes of an IO vector
> - * @iov:	IO vector
> + * iov_skip_bytes() - Find index and offset in iovec array given byte offset
> + * @iov:	iovec array
>   * @n:		Number of entries in @iov
> - * @skip:	Number of leading bytes of @iov to skip
> - * @offset:	Offset of first unskipped byte in its @iov entry
> + * @skip:	Byte offset: leading bytes of @iov to skip
> + * @offset:	Offset within matching @iov entry, set on return, can be NULL
>   *
> - * Return: index I of individual struct iovec which contains the byte at @skip
> - *         bytes into the vector (as though all its buffers were contiguous).
> - *         If @offset is non-NULL, update it to the offset of that byte within
> - *         @iov[I] (guaranteed to be less than @iov[I].iov_len) If the whole
> - *         vector has <= @skip bytes, return @n.
> + * Return: index of iovec array containing the @skip byte counted as if buffers
> + *	   were contiguous. If iovec has less than @skip bytes, return @n.
>   */
>  size_t iov_skip_bytes(const struct iovec *iov, size_t n,
>  		      size_t skip, size_t *offset)
> @@ -57,17 +54,14 @@ size_t iov_skip_bytes(const struct iovec *iov, size_t n,
>  }
>  
>  /**
> - * iov_from_buf() - Copy data from a buffer to an I/O vector (struct iovec)
> - *                  efficiently.
> - *
> - * @iov:       Pointer to the array of struct iovec describing the
> - *             scatter/gather I/O vector.
> - * @iov_cnt:   Number of elements in the iov array.
> - * @offset:    Byte offset in the iov array where copying should start.
> - * @buf:       Pointer to the source buffer containing the data to copy.
> - * @bytes:     Total number of bytes to copy from buf to iov.
> + * iov_from_buf() - Copy from flat buffer to iovec array
> + * @iov:	Destination iovec array
> + * @iov_cnt:	Number of elements in the iovec array
> + * @offset:	Destination offset in iovec array

Nit: Any reason not to mention the "counted as if .. contiguous" like
you have for iov_to_buf()?

> + * @buf:	Source buffer
> + * @bytes:	Bytes to copy
>   *
> - * Return: the number of bytes successfully copied.
> + * Return: number of bytes copied
>   */
>  size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>  		    size_t offset, const void *buf, size_t bytes)
> @@ -78,12 +72,12 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>  	if (__builtin_constant_p(bytes) && iov_cnt &&
>  		offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
>  		memcpy((char *)iov[0].iov_base + offset, buf, bytes);
> +
>  		return bytes;
>  	}
>  
>  	i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
>  
> -	/* copying data */
>  	for (copied = 0; copied < bytes && i < iov_cnt; i++) {
>  		size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
>  
> @@ -97,17 +91,14 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>  }
>  
>  /**
> - * iov_to_buf() - Copy data from a scatter/gather I/O vector (struct iovec) to
> - *		  a buffer efficiently.
> - *
> - * @iov:       Pointer to the array of struct iovec describing the scatter/gather
> - *             I/O vector.
> - * @iov_cnt:   Number of elements in the iov array.
> - * @offset:    Offset within the first element of iov from where copying should start.
> - * @buf:       Pointer to the destination buffer where data will be copied.
> - * @bytes:     Total number of bytes to copy from iov to buf.
> + * iov_to_buf() - Copy from iovec to flat buffer
> + * @iov:	Source iovec array
> + * @iov_cnt:	Number of elements in iovec array
> + * @offset:	Source offset in @iov, counted as if buffers were contiguous
> + * @buf:	Destination buffer
> + * @bytes:	Bytes to copy
>   *
> - * Return: the number of bytes successfully copied.
> + * Return: number of bytes copied
>   */
>  size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>  		  size_t offset, void *buf, size_t bytes)
> @@ -118,15 +109,17 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>  	if (__builtin_constant_p(bytes) && iov_cnt &&
>  		offset <= iov[0].iov_len && bytes <= iov[0].iov_len - offset) {
>  		memcpy(buf, (char *)iov[0].iov_base + offset, bytes);
> +
>  		return bytes;
>  	}
>  
>  	i = iov_skip_bytes(iov, iov_cnt, offset, &offset);
>  
> -	/* copying data */
>  	for (copied = 0; copied < bytes && i < iov_cnt; i++) {
>  		size_t len = MIN(iov[i].iov_len - offset, bytes - copied);
> +
>  		ASSERT(iov[i].iov_base);
> +
>  		memcpy((char *)buf + copied, (char *)iov[i].iov_base + offset,
>  		       len);
>  		copied += len;
> @@ -137,14 +130,11 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>  }
>  
>  /**
> - * iov_size() - Calculate the total size of a scatter/gather I/O vector
> - *              (struct iovec).
> + * iov_size() - Calculate total data size of iovec
> + * @iov:	Source iovec array
> + * @iov_cnt:	Number of elements in iovec array
>   *
> - * @iov:       Pointer to the array of struct iovec describing the
> - *             scatter/gather I/O vector.
> - * @iov_cnt:   Number of elements in the iov array.
> - *
> - * Return: the total size in bytes.
> + * Return: total size in bytes
>   */
>  size_t iov_size(const struct iovec *iov, size_t iov_cnt)
>  {
> -- 
> 2.43.0
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] iov: Fix coding style of basic (non-IOV_TAIL) parts

[Stefano Brivio]

On Fri, 5 Dec 2025 13:58:53 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Dec 05, 2025 at 01:51:11AM +0100, Stefano Brivio wrote:
> > I happened to touch these functions for a change that turned out to be
> > unnecessary, but coding style and documentation could still benefit
> > from some rephrasing and fixes.
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> One nit below..
> 
> > ---
> >  iov.c | 68 +++++++++++++++++++++++++----------------------------------
> >  1 file changed, 29 insertions(+), 39 deletions(-)
> > 
> > diff --git a/iov.c b/iov.c
> > index 8c74a59..d31464d 100644
> > --- a/iov.c
> > +++ b/iov.c
> > @@ -20,24 +20,21 @@
> >   * Contributions after 2012-01-13 are licensed under the terms of the
> >   * GNU GPL, version 2 or (at your option) any later version.
> >   */
> > +
> >  #include <sys/socket.h>
> >  
> >  #include "util.h"
> >  #include "iov.h"
> >  
> > -
> >  /**
> > - * iov_skip_bytes() - Skip leading bytes of an IO vector
> > - * @iov:	IO vector
> > + * iov_skip_bytes() - Find index and offset in iovec array given byte offset
> > + * @iov:	iovec array
> >   * @n:		Number of entries in @iov
> > - * @skip:	Number of leading bytes of @iov to skip
> > - * @offset:	Offset of first unskipped byte in its @iov entry
> > + * @skip:	Byte offset: leading bytes of @iov to skip
> > + * @offset:	Offset within matching @iov entry, set on return, can be NULL
> >   *
> > - * Return: index I of individual struct iovec which contains the byte at @skip
> > - *         bytes into the vector (as though all its buffers were contiguous).
> > - *         If @offset is non-NULL, update it to the offset of that byte within
> > - *         @iov[I] (guaranteed to be less than @iov[I].iov_len) If the whole
> > - *         vector has <= @skip bytes, return @n.
> > + * Return: index of iovec array containing the @skip byte counted as if buffers
> > + *	   were contiguous. If iovec has less than @skip bytes, return @n.
> >   */
> >  size_t iov_skip_bytes(const struct iovec *iov, size_t n,
> >  		      size_t skip, size_t *offset)
> > @@ -57,17 +54,14 @@ size_t iov_skip_bytes(const struct iovec *iov, size_t n,
> >  }
> >  
> >  /**
> > - * iov_from_buf() - Copy data from a buffer to an I/O vector (struct iovec)
> > - *                  efficiently.
> > - *
> > - * @iov:       Pointer to the array of struct iovec describing the
> > - *             scatter/gather I/O vector.
> > - * @iov_cnt:   Number of elements in the iov array.
> > - * @offset:    Byte offset in the iov array where copying should start.
> > - * @buf:       Pointer to the source buffer containing the data to copy.
> > - * @bytes:     Total number of bytes to copy from buf to iov.
> > + * iov_from_buf() - Copy from flat buffer to iovec array
> > + * @iov:	Destination iovec array
> > + * @iov_cnt:	Number of elements in the iovec array
> > + * @offset:	Destination offset in iovec array  
> 
> Nit: Any reason not to mention the "counted as if .. contiguous" like
> you have for iov_to_buf()?

Earlier, I didn't specify that as it made the comment too long, but
then I changed "iovec array" to "@iov" in the comment to iov_to_buf(),
and I didn't realise I could do the same here.

Changed, let me re-spin this as it's all about comments and coding
style anyway.

-- 
Stefano