Re: [PATCH v8 2/3] iov: Introduce IOV_PUSH_HEADER() macro

[Laurent Vivier <lvivier@redhat.com>]

On 5/10/26 00:15, Jon Maloy wrote:
> 
> 
> On 2026-04-16 12:09, Laurent Vivier wrote:
>> Add iov_push_header_() and its typed wrapper IOV_PUSH_HEADER() to write
>> a header into an iov_tail at the current offset and advance past it.
>>
>> This is the write counterpart to IOV_PEEK_HEADER() / IOV_REMOVE_HEADER(),
>> using iov_from_buf() to copy the header data across iovec boundaries.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>> ---
>>   iov.c | 23 +++++++++++++++++++++++
>>   iov.h | 11 +++++++++++
>>   2 files changed, 34 insertions(+)
>>
>> diff --git a/iov.c b/iov.c
>> index 28c6d40d2986..b1bcdc4649df 100644
>> --- a/iov.c
>> +++ b/iov.c
>> @@ -360,6 +360,29 @@ void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, 
>> size_t align)
>>       return v;
>>   }
>> +/**
>> + * iov_push_header_() - Write a new header to an IOV tail
>> + * @tail:    IOV tail to write header to
>> + * @v:        Pointer to header data to write
>> + * @len:    Length of header to write, in bytes
>> + *
>> + * Return: number of bytes written
>> + */
>> +/* cppcheck-suppress unusedFunction */
>> +size_t iov_push_header_(struct iov_tail *tail, const void *v, size_t len)
>> +{
>> +    size_t l;
>> +
>> +    if (!iov_tail_prune(tail))
>> +        return 0; /* No space */
>> +
>> +    l = iov_from_buf(tail->iov, tail->cnt, tail->off, v, len);
>> +
>> +    tail->off = tail->off + l;
>> +
>> +    return l;
>> +}
> 
> A small observation: if iov_from_buf() returns less than ´len' because the tail has 
> insufficient space, this function advances tail->off by that lenght and returns it. The 
> caller in IOV_PUSH_HEADER gets back a
> (value != sizeof(header)), but ignores it. This means a partial header write would go 
> undetected.
> Maybe a warning or even an assert() would be in place here?
> /jon

I'll add a warning here. I'd rather not use an assert because
I want to keep the function flexible enough to handle short writes
gracefully. In principle, the caller should check the return value, but
in practice all current callers already know the buffer is large enough
for the header, so adding that check would just clutter the code.

Thanks,
Laurent

> 
>> +
>>   /**
>>    * iov_remove_header_() - Remove a header from an IOV tail
>>    * @tail:    IOV tail to remove header from (modified)
>> diff --git a/iov.h b/iov.h
>> index 3c63308e554f..4fdf14a85b19 100644
>> --- a/iov.h
>> +++ b/iov.h
>> @@ -93,6 +93,7 @@ bool iov_tail_prune(struct iov_tail *tail);
>>   size_t iov_tail_size(struct iov_tail *tail);
>>   bool iov_drop_header(struct iov_tail *tail, size_t len);
>>   void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align);
>> +size_t iov_push_header_(struct iov_tail *tail, const void *v, size_t len);
>>   void *iov_remove_header_(struct iov_tail *tail, void *v, size_t len, size_t align);
>>   ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
>>                  struct iov_tail *tail);
>> @@ -115,6 +116,16 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
>>                              sizeof(var_),        \
>>                              __alignof__(var_))))
>> +/**
>> + * IOV_PUSH_HEADER() - Write a new header to an IOV tail
>> + * @tail_:    IOV tail to write header to
>> + * @var_:    A variable containing the header data to write
>> + *
>> + * Return: number of bytes written
>> + */
>> +#define IOV_PUSH_HEADER(tail_, var_)                    \
>> +    (iov_push_header_((tail_), &(var_), sizeof(var_)))
>> +
>>   /**
>>    * IOV_REMOVE_HEADER() - Remove and return typed header from an IOV tail
>>    * @tail_:    IOV tail to remove header from (modified)
>