On Fri, Aug 08, 2025 at 04:01:15PM +0200, Laurent Vivier wrote: > Provide a temporary variable of the wanted type to store > the header if the memory in the iovec array is not contiguous. > > Signed-off-by: Laurent Vivier > --- > iov.c | 55 +++++++++++++++++++++++++++++++++++++++++++++---------- > iov.h | 55 +++++++++++++++++++++++++++++++++++++++++-------------- > tcp_buf.c | 2 +- > 3 files changed, 87 insertions(+), 25 deletions(-) > > diff --git a/iov.c b/iov.c > index 9d282d4af461..d39bb099fa69 100644 > --- a/iov.c > +++ b/iov.c > @@ -109,7 +109,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt, > * > * Return: the number of bytes successfully copied. > */ > -/* cppcheck-suppress unusedFunction */ > +/* cppcheck-suppress [staticFunction] */ > size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, > size_t offset, void *buf, size_t bytes) > { > @@ -127,6 +127,7 @@ size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt, > /* copying data */ > for (copied = 0; copied < bytes && i < iov_cnt; i++) { > size_t len = MIN(iov[i].iov_len - offset, bytes - copied); > + /* NOLINTNEXTLINE(clang-analyzer-core.NonNullParamChecker) */ This suppression worries me slightly - I don't really understand why clang would be complaining here in the first place. [snip] > @@ -275,7 +310,7 @@ void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align) > * > * Return: the number of elements successfully referenced from the destination > * iov array, a negative value if there is not enough room in the > - * destination iov array > + * destination iov array Looks like a spurious whitespace change. > */ > /* cppcheck-suppress unusedFunction */ > ssize_t iov_tail_clone(struct iovec *dst_iov, size_t dst_iov_cnt, > diff --git a/iov.h b/iov.h > index bf9820ac52ab..ccdb690ef3f1 100644 > --- a/iov.h > +++ b/iov.h > @@ -70,41 +70,68 @@ struct iov_tail { > #define IOV_TAIL(iov_, cnt_, off_) \ > (struct iov_tail){ .iov = (iov_), .cnt = (cnt_), .off = (off_) } > > +/** > + * IOV_TAIL_FROM_BUF() - Create a new IOV tail from a buffer > + * @buf_: Buffer address to use in the iovec > + * @len_: Buffer size > + * @off_: Byte offset in the buffer where the tail begins > + */ > +#define IOV_TAIL_FROM_BUF(buf_, len_, off_) \ > + IOV_TAIL((&(const struct iovec){ .iov_base = (buf_), \ > + .iov_len = (len_) }), \ > + 1, \ > + (off_)) I think this belongs in the next patch instead. > bool iov_tail_prune(struct iov_tail *tail); > size_t iov_tail_size(struct iov_tail *tail); > bool iov_tail_drop(struct iov_tail *tail, size_t len); > -void *iov_peek_header_(struct iov_tail *tail, size_t len, size_t align); > -void *iov_remove_header_(struct iov_tail *tail, size_t len, size_t align); > +void *iov_peek_header_(struct iov_tail *tail, void *v, size_t len, size_t align); > +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); > > /** > * IOV_PEEK_HEADER() - Get typed pointer to a header from an IOV tail > * @tail_: IOV tail to get header from > - * @type_: Data type of the header > + * @var_: Temporary buffer of the type of the header to use if > + * the memory in the iovec array is not contiguous. > * > * @tail_ may be pruned, but will represent the same bytes as before. > * > - * Return: pointer of type (@type_ *) located at the start of @tail_, NULL if > - * we can't get a contiguous and aligned pointer. > + * Return: pointer of type (@type_ *) located at the start of @tail_ > + * or to @var_ if iovec memory is not contiguous, NULL if > + * that overruns the iovec. > */ > -#define IOV_PEEK_HEADER(tail_, type_) \ > - ((type_ *)(iov_peek_header_((tail_), \ > - sizeof(type_), __alignof__(type_)))) > + > +#define IOV_PEEK_HEADER(tail_, var_) \ > + ((__typeof__(var_) *)(iov_peek_header_((tail_), &(var_), \ > + sizeof(var_), \ > + __alignof__(var_)))) > > /** > * IOV_REMOVE_HEADER() - Remove and return typed header from an IOV tail > * @tail_: IOV tail to remove header from (modified) > - * @type_: Data type of the header to remove > + * @var_: Temporary buffer of the type of the header to use if > + * the memory in the iovec array is not contiguous. > * > * On success, @tail_ is updated so that it longer includes the bytes of the > * returned header. > * > - * Return: pointer of type (@type_ *) located at the old start of @tail_, NULL > - * if we can't get a contiguous and aligned pointer. > + * Return: pointer of type (@type_ *) located at the start of @tail_ > + * or to @var_ if iovec memory is not contiguous, NULL if > + * that overruns the iovec. > + */ > + > +#define IOV_REMOVE_HEADER(tail_, var_) \ > + ((__typeof__(var_) *)(iov_remove_header_((tail_), &(var_), \ > + sizeof(var_), __alignof__(var_)))) > + > +/** 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 > + * > + * Return: true if the tail still contains any bytes, otherwise false > */ > -#define IOV_REMOVE_HEADER(tail_, type_) \ > - ((type_ *)(iov_remove_header_((tail_), \ > - sizeof(type_), __alignof__(type_)))) > +#define IOV_DROP_HEADER(tail_, type_) iov_tail_drop((tail_), sizeof(type_)) > > #endif /* IOVEC_H */ > diff --git a/tcp_buf.c b/tcp_buf.c > index d1fca676c9a7..bc898de86919 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -160,7 +160,7 @@ static void tcp_l2_buf_fill_headers(const struct tcp_tap_conn *conn, > uint32_t seq, bool no_tcp_csum) > { > struct iov_tail tail = IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0); > - struct tcphdr *th = IOV_REMOVE_HEADER(&tail, struct tcphdr); > + struct tcphdr th_storage, *th = IOV_REMOVE_HEADER(&tail, th_storage); > struct tap_hdr *taph = iov[TCP_IOV_TAP].iov_base; > const struct flowside *tapside = TAPFLOW(conn); > const struct in_addr *a4 = inany_v4(&tapside->oaddr); -- 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