RE: [PATCH v2] vhost: fix build

["Xia, Chenbo" <chenbo.xia@intel.com>]

Hi Min,

> -----Original Message-----
> From: Min Zhou <zhoumin@loongson.cn>
> Sent: Monday, August 29, 2022 4:29 PM
> To: david.marchand@redhat.com; maxime.coquelin@redhat.com; Xia, Chenbo
> <chenbo.xia@intel.com>; zhoumin@loongson.cn
> Cc: dev@dpdk.org; maobibo@loongson.cn
> Subject: [PATCH v2] vhost: fix build
> 
> On CentOS 8 or Debian 10.4 systems using gcc 12.1 to cross
> compile DPDK, gcc shows a following warning which will cause
> build to fail when build is run with -werror:
> 
> In function 'mbuf_to_desc',
>     inlined from 'vhost_enqueue_async_packed'
> at ../lib/vhost/virtio_net.c:1826:6,
>     inlined from 'virtio_dev_rx_async_packed'
> at ../lib/vhost/virtio_net.c:1840:6,
>     inlined from 'virtio_dev_rx_async_submit_packed.constprop'
> at ../lib/vhost/virtio_net.c:1900:7:
> ../lib/vhost/virtio_net.c:1161:35: error: 'buf_vec[0].buf_len' may be used
> uninitialized [-Werror=maybe-uninitialized]
>  1161 |         buf_len = buf_vec[vec_idx].buf_len;
>       |                   ~~~~~~~~~~~~~~~~^~~~~~~~
> ../lib/vhost/virtio_net.c: In function
> 'virtio_dev_rx_async_submit_packed.constprop':
> ../lib/vhost/virtio_net.c:1838:27: note: 'buf_vec' declared here
>  1838 |         struct buf_vector buf_vec[BUF_VECTOR_MAX];
>       |                           ^~~~~~~
> cc1: all warnings being treated as errors
> 
> Actually, there are eight places to see the same codes in the file
> lib/vhost/virtio_net.c, and all these `buf_vec` arraies are
> initialized by sub-function calls under various conditions.
> 
> Although It's hard to understand why gcc just emits warning at one
> of the eight places, adding validity checks for array length is
> reasonable and can also fix the warning.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
> ---
>  lib/vhost/virtio_net.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Just want you to know that your patch is still pending because by accident
your fix is almost the same as a previous patch that fixes a real issue but
that patch is still in progress:

http://patchwork.dpdk.org/project/dpdk/patch/20220802004938.23670-2-cfontana@suse.de/

Thanks,
Chenbo

> 
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 35fa4670fd..99233f1759 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -1153,7 +1153,7 @@ mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  	struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
>  	struct vhost_async *async = vq->async;
> 
> -	if (unlikely(m == NULL))
> +	if (unlikely(m == NULL || nr_vec == 0))
>  		return -1;
> 
>  	buf_addr = buf_vec[vec_idx].buf_addr;
> @@ -2673,6 +2673,9 @@ desc_to_mbuf(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
>  	struct vhost_async *async = vq->async;
>  	struct async_inflight_info *pkts_info;
> 
> +	if (unlikely(nr_vec == 0))
> +		return -1;
> +
>  	buf_addr = buf_vec[vec_idx].buf_addr;
>  	buf_iova = buf_vec[vec_idx].buf_iova;
>  	buf_len = buf_vec[vec_idx].buf_len;
> --
> 2.31.1