Re: [PATCH v5 07/29] arp: Convert to iov_tail

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 17 Apr 2025 18:51:14 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Use packet_data() and extract headers using IOV_REMOVE_HEADER()
> rather than packet_get().
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  arp.c    | 12 +++++++++---
>  packet.c |  1 -
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/arp.c b/arp.c
> index 9d68d7c3b602..4cb6965e9793 100644
> --- a/arp.c
> +++ b/arp.c
> @@ -77,11 +77,17 @@ int arp(const struct ctx *c, const struct pool *p)
>  	const struct ethhdr *eh;
>  	const struct arphdr *ah;
>  	const struct arpmsg *am;
> +	struct iov_tail data;
> +	struct arphdr  ahc;

Nit: excess whitespace.

> +	struct ethhdr ehc;
> +	struct arpmsg amc;
>  
> -	eh = packet_get(p, 0, 0,			 sizeof(*eh), NULL);
> -	ah = packet_get(p, 0, sizeof(*eh),		 sizeof(*ah), NULL);
> -	am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL);
> +	if (!packet_data(p, 0, &data))
> +		return -1;
>  
> +	eh = IOV_REMOVE_HEADER(&data, ehc);
> +	ah = IOV_REMOVE_HEADER(&data, ahc);
> +	am = IOV_REMOVE_HEADER(&data, amc);

That's quite neat!

>  	if (!eh || !ah || !am)
>  		return -1;
>  
> diff --git a/packet.c b/packet.c
> index 675bccf4d787..b6de869db176 100644
> --- a/packet.c
> +++ b/packet.c
> @@ -200,7 +200,6 @@ void *packet_get_do(const struct pool *p, const size_t idx,
>   *
>   * Return: true if @data contains valid data, false otherwise
>   */
> -/* cppcheck-suppress unusedFunction */
>  bool packet_data_do(const struct pool *p, size_t idx,
>  		    struct iov_tail *data,
>  		    const char *func, int line)

-- 
Stefano