Re: [PATCH v4 4/5] iov: Add IOV_PUT_HEADER() and with_header() to write header data back to iov_tail

[David Gibson <david@gibson.dropbear.id.au>]

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

On Tue, Mar 24, 2026 at 08:16:50AM +0100, Laurent Vivier wrote:
> On 3/24/26 03:41, David Gibson wrote:
> > On Mon, Mar 23, 2026 at 03:31:50PM +0100, Laurent Vivier wrote:
> > > Add iov_put_header_() and its wrapper macro IOV_PUT_HEADER() as a
> > > counterpart to IOV_PEEK_HEADER(). This writes header data back to an
> > > iov_tail after modification. If the header pointer matches the
> > > original iov buffer location, the data was already modified in place
> > > and no copy is needed. Otherwise, it copies the data back using
> > > iov_from_buf().
> > > 
> > > Add with_header(), a for-loop macro that combines IOV_PEEK_HEADER()
> > > and IOV_PUT_HEADER() to allow modifying a header in place within a
> > > block scope.
> > > 
> > > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > > ---
> > >   iov.c | 22 ++++++++++++++++++++++
> > >   iov.h | 25 ++++++++++++++++++++++++-
> > >   2 files changed, 46 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/iov.c b/iov.c
> > > index 8134b8c9f988..7fc9c3c78a32 100644
> > > --- a/iov.c
> > > +++ b/iov.c
> > > @@ -308,6 +308,28 @@ void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align)
> > >   	return v;
> > >   }
> > > +/**
> > > + * iov_put_header_() - Write header back 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_put_header_(const struct iov_tail *tail, const void *v, size_t len)
> > > +{
> > > +	size_t l = len;
> > > +
> > > +	/* iov_peek_header_() already called iov_check_header() */
> > > +	if ((char *)tail->iov[0].iov_base + tail->off != v)
> > > +		l = iov_from_buf(tail->iov, tail->cnt, tail->off, v, len);
> > > +
> > > +	assert(l == len);
> > > +
> > > +	return l;
> > > +}
> > > +
> > >   /**
> > >    * 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 d295d05b3bab..4ce425ccdbe5 100644
> > > --- a/iov.h
> > > +++ b/iov.h
> > > @@ -90,6 +90,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_put_header_(const 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);
> > > @@ -112,6 +113,16 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
> > >   					       sizeof(var_),		\
> > >   					       __alignof__(var_))))
> > > +/**
> > > + * IOV_PUT_HEADER() - Write header back to an IOV tail
> > > + * @tail_:	IOV tail to write header to
> > > + * @var_:	Pointer to a variable containing the header data to write
> > > + *
> > > + * Return: number of bytes written
> > > + */
> > > +#define IOV_PUT_HEADER(tail_, var_)					\
> > > +	(iov_put_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)
> > > @@ -130,7 +141,8 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
> > >   	((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_),	\
> > >   				    sizeof(var_), __alignof__(var_))))
> > > -/** IOV_DROP_HEADER() - Remove a typed header from an IOV tail
> > > +/**
> > > + * IOV_DROP_HEADER() - Remove a typed header from an IOV tail
> > >    * @tail_:	IOV tail to remove header from (modified)
> > >    * @type_:	Data type of the header to remove
> > >    *
> > > @@ -138,4 +150,15 @@ ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt,
> > >    */
> > >   #define IOV_DROP_HEADER(tail_, type_) 	iov_drop_header((tail_), sizeof(type_))
> > > +/**
> > > + * with_header() - Execute a block on a given header
> > > + * @type:	Data type of the header to modify
> > 
> > We already use __typeof__ in IOV_PEEK_HEADER(), so we should be able
> > to use that to avoid explicitly passing the type.
> 
> I tried but it's not possible to put a declaration and an initialization in
> the first part of the for().
> 
> for instance
> 
> #define with_header(hdr_, tail_)\
> for (__typeof__(*hdr_) store_, hdr_ = IOV_PEEK_HEADER(tail_, store_);\
>      hdr_;\
>      IOV_PUT_HEADER(tail_, hdr_), hdr_ = NULL)
> 
> Then :
> 
> struct ethhdr *eh;
> struct iov_tail *tail;
> 
> with_header(eh, tail)
> 
> becomes:
> 
> for (struct ethhdr store_, eh = IOV_PEEK_HEADER(tail, store_);
>      eh;
>      IOV_PUT_HEADER(store, eh), eh = NULL)

Huh, right.  I'd never realised that limitation.

> And
> 
> struct ethhdr store_, eh = IOV_PEEK_HEADER(tail, store_);
> 
> declares store_, but also declares eh, that is already declared.
> 
> We could do something like:
> 
> #define with_header(hdr_, tail_)\
> for (__typeof__(*hdr_) store_, *tmp_hdr_ = (hdr_ = IOV_PEEK_HEADER(tail_, store_));\
>      hdr_;\
>      IOV_PUT_HEADER(tail_, hdr_), hdr_ = NULL)
> 
> But I preferred to introduce the type rather than a dummy variable.

Hmm.  I actually prefer this second solution.  It's ugly inside the
macro, but it makes it neater for the caller.  The temporary is never
used, so should get pruned by the compiler (although I guess we need
to see if it makes cppcheck unhappy.

> Moreover, doing as I did, the variable is local to the body of the for() and
> cannot be used outside.

Uh.. that should be true for both variants.


-- 
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 --]