* Re: [PATCH v2] vhost: fix build
2022-08-29 8:29 [PATCH v2] vhost: fix build Min Zhou
@ 2022-09-01 0:06 ` zhoumin
2022-09-09 9:07 ` zhoumin
2022-09-26 2:57 ` Xia, Chenbo
2 siblings, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-09-01 0:06 UTC (permalink / raw)
To: david.marchand, maxime.coquelin, chenbo.xia; +Cc: dev, maobibo
Sincerely ping.
On 2022年08月29日 16:29, Min Zhou wrote:
> 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(-)
>
> 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;
--
Thanks,
Min Zhou
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] vhost: fix build
2022-08-29 8:29 [PATCH v2] vhost: fix build Min Zhou
2022-09-01 0:06 ` zhoumin
@ 2022-09-09 9:07 ` zhoumin
2022-09-26 2:57 ` Xia, Chenbo
2 siblings, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-09-09 9:07 UTC (permalink / raw)
To: david.marchand, maxime.coquelin, chenbo.xia; +Cc: dev, maobibo
Kindly ping.
Any feedback is appreciated.
On 2022年08月29日 16:29, Min Zhou wrote:
> 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(-)
>
> 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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2] vhost: fix build
2022-08-29 8:29 [PATCH v2] vhost: fix build Min Zhou
2022-09-01 0:06 ` zhoumin
2022-09-09 9:07 ` zhoumin
@ 2022-09-26 2:57 ` Xia, Chenbo
2022-09-26 3:25 ` zhoumin
2 siblings, 1 reply; 8+ messages in thread
From: Xia, Chenbo @ 2022-09-26 2:57 UTC (permalink / raw)
To: Min Zhou, david.marchand, maxime.coquelin; +Cc: dev, maobibo
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] vhost: fix build
2022-09-26 2:57 ` Xia, Chenbo
@ 2022-09-26 3:25 ` zhoumin
2022-10-05 14:51 ` Maxime Coquelin
0 siblings, 1 reply; 8+ messages in thread
From: zhoumin @ 2022-09-26 3:25 UTC (permalink / raw)
To: Xia, Chenbo, david.marchand, maxime.coquelin; +Cc: dev, maobibo
Hi Chenbo,
On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
> 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
Thanks for your helpful reply.
I think I can drop this patch if the patch you mentioned above could be
accepted.
Thanks,
Min Zhou
>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] vhost: fix build
2022-09-26 3:25 ` zhoumin
@ 2022-10-05 14:51 ` Maxime Coquelin
2022-10-06 3:56 ` zhoumin
2022-10-06 3:57 ` zhoumin
0 siblings, 2 replies; 8+ messages in thread
From: Maxime Coquelin @ 2022-10-05 14:51 UTC (permalink / raw)
To: zhoumin, Xia, Chenbo, david.marchand; +Cc: dev, maobibo
Hi Min,
On 9/26/22 05:25, zhoumin wrote:
> Hi Chenbo,
>
>
> On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
>> 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
>
> Thanks for your helpful reply.
> I think I can drop this patch if the patch you mentioned above could be
> accepted.
I just sent another patch that fixes your build issue without having to
add runtime checks:
http://patches.dpdk.org/project/dpdk/patch/20221005144859.70717-1-maxime.coquelin@redhat.com/
Thanks,
Maxime
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] vhost: fix build
2022-10-05 14:51 ` Maxime Coquelin
@ 2022-10-06 3:56 ` zhoumin
2022-10-06 3:57 ` zhoumin
1 sibling, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-10-06 3:56 UTC (permalink / raw)
To: Maxime Coquelin, Xia, Chenbo, david.marchand; +Cc: dev, maobibo
Hi, Maxime,
Thanks a lot for your kind reply.
This patch is very useful and relieves the concerns in my heart.
On Wed, Oct 5, 2022 at 22:51, Maxime Coquelin wrote:
> Hi Min,
>
> On 9/26/22 05:25, zhoumin wrote:
>> Hi Chenbo,
>>
>>
>> On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
>>> 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
>>
>> Thanks for your helpful reply.
>> I think I can drop this patch if the patch you mentioned above could
>> be accepted.
>
> I just sent another patch that fixes your build issue without having to
> add runtime checks:
>
> http://patches.dpdk.org/project/dpdk/patch/20221005144859.70717-1-maxime.coquelin@redhat.com/
>
>
OK, thanks. I have read the whole thread, which is a good job. I'd like
thank you all for your hard work.
> Thanks,
> Maxime
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] vhost: fix build
2022-10-05 14:51 ` Maxime Coquelin
2022-10-06 3:56 ` zhoumin
@ 2022-10-06 3:57 ` zhoumin
1 sibling, 0 replies; 8+ messages in thread
From: zhoumin @ 2022-10-06 3:57 UTC (permalink / raw)
To: Maxime Coquelin, Xia, Chenbo, david.marchand; +Cc: dev, maobibo
Hi, Maxime,
Thanks a lot for your kind reply.
This patch is very useful and relieves the concerns in my heart.
On Wed, Oct 5, 2022 at 22:51, Maxime Coquelin wrote:
> Hi Min,
>
> On 9/26/22 05:25, zhoumin wrote:
>> Hi Chenbo,
>>
>>
>> On Mon, 26 Sep 2022, 10:57, Xia, Chenbo wrote:
>>> 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
>>
>> Thanks for your helpful reply.
>> I think I can drop this patch if the patch you mentioned above could
>> be accepted.
>
> I just sent another patch that fixes your build issue without having to
> add runtime checks:
>
> http://patches.dpdk.org/project/dpdk/patch/20221005144859.70717-1-maxime.coquelin@redhat.com/
>
>
OK, thanks. I have read the whole thread, which is a good job. I'd like
thank you all for your hard work.
> Thanks,
> Maxime
>
Thanks,
Min Zhou
^ permalink raw reply [flat|nested] 8+ messages in thread