Re: [PATCH 01/24] iov: add some functions to manage iovec

On 2/7/24 02:01, David Gibson wrote:
...
  On Tue, Feb 06, 2024 at 03:28:10PM +0100, Laurent
Vivier wrote:
  On 2/5/24 06:57, David Gibson wrote:
  On Fri, Feb 02, 2024 at 03:11:28PM +0100, Laurent
Vivier wrote:
 ...
  diff --git a/iov.c b/iov.c
 new file mode 100644
 index 000000000000..38a8e7566021
 --- /dev/null
 +++ b/iov.c

 +	for (i = 0, done = 0; (offset || done < bytes) && i < iov_cnt; i++) {
 Not immediately seeing why you need the 'offset ||' part of the
condition. 
 In fact the loop has two purposes:

 1- scan the the iovec to reach byte offset in the iov (so until offset is 0)

 2- copy the bytes (until done == byte)

 It could be written like this:

 for (i = 0; offset && i < iov_cnt && offset >= iov[i].iov_len ;
i++)
          offset -= iov[i].iov_len;

 for (done = 0; done < bytes && i < iov_cnt; i++) {
      size_t len = MIN(iov[i].iov_len - offset, bytes - done);
      memcpy((char *)iov[i].iov_base + offset, (char *)buf + done, len);
      done += len;
 } 
 Right, but done starts at 0 and will remain zero until you reach the
 first segment where you need to copy something.  So, unless bytes ==
 0, then done < bytes will always be true when offset != 0.  And if
 bytes *is* zero, then there's nothing to do, so there's no need to
 step through the vector at all.
 
Yes, you're right. I'm going to remove the "offset" test from
the condition.

Thanks,
Laurent