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

[Laurent Vivier <lvivier@redhat.com>]

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