* [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning
@ 2023-09-11 21:54 Stefan Hajnoczi
2023-09-12 6:18 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-09-11 21:54 UTC (permalink / raw)
To: qemu-devel
Cc: Jason Wang, Stefan Hajnoczi, Eugenio Pérez,
Michael S . Tsirkin
gcc 13.2.1 emits the following warning:
net/vhost-vdpa.c: In function ‘net_vhost_vdpa_init.constprop’:
net/vhost-vdpa.c:1394:25: error: ‘cvq_isolated’ may be used uninitialized [-Werror=maybe-uninitialized]
1394 | s->cvq_isolated = cvq_isolated;
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
net/vhost-vdpa.c:1355:9: note: ‘cvq_isolated’ was declared here
1355 | int cvq_isolated;
| ^~~~~~~~~~~~
cc1: all warnings being treated as errors
Cc: Eugenio Pérez <eperezma@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
net/vhost-vdpa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 34202ca009..7eaee841aa 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -1352,7 +1352,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
VhostVDPAState *s;
int ret = 0;
assert(name);
- int cvq_isolated;
+ int cvq_isolated = 0;
if (is_datapath) {
nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
--
2.41.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning
2023-09-11 21:54 [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning Stefan Hajnoczi
@ 2023-09-12 6:18 ` Philippe Mathieu-Daudé
2023-09-12 10:48 ` Stefan Hajnoczi
2023-09-12 6:46 ` Eugenio Perez Martin
2023-09-15 5:30 ` Jason Wang
2 siblings, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-12 6:18 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Jason Wang, Eugenio Pérez, Michael S . Tsirkin
On 11/9/23 23:54, Stefan Hajnoczi wrote:
> gcc 13.2.1 emits the following warning:
>
> net/vhost-vdpa.c: In function ‘net_vhost_vdpa_init.constprop’:
> net/vhost-vdpa.c:1394:25: error: ‘cvq_isolated’ may be used uninitialized [-Werror=maybe-uninitialized]
> 1394 | s->cvq_isolated = cvq_isolated;
> | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> net/vhost-vdpa.c:1355:9: note: ‘cvq_isolated’ was declared here
> 1355 | int cvq_isolated;
> | ^~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Cc: Eugenio Pérez <eperezma@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> net/vhost-vdpa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 34202ca009..7eaee841aa 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1352,7 +1352,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
> VhostVDPAState *s;
> int ret = 0;
> assert(name);
> - int cvq_isolated;
> + int cvq_isolated = 0;
>
> if (is_datapath) {
> nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
Alternatively:
-- >8 --
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 34202ca009..218fe0c305 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -1352,13 +1352,12 @@ static NetClientState
*net_vhost_vdpa_init(NetClientState *peer,
VhostVDPAState *s;
int ret = 0;
assert(name);
- int cvq_isolated;
if (is_datapath) {
nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
name);
} else {
- cvq_isolated = vhost_vdpa_probe_cvq_isolation(vdpa_device_fd,
features,
+ int cvq_isolated =
vhost_vdpa_probe_cvq_isolation(vdpa_device_fd, features,
queue_pair_index
* 2,
errp);
if (unlikely(cvq_isolated < 0)) {
@@ -1391,7 +1390,7 @@ static NetClientState
*net_vhost_vdpa_init(NetClientState *peer,
s->vhost_vdpa.shadow_vq_ops = &vhost_vdpa_net_svq_ops;
s->vhost_vdpa.shadow_vq_ops_opaque = s;
- s->cvq_isolated = cvq_isolated;
+ s->cvq_isolated = true;
---
Whichever you prefer:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning
2023-09-11 21:54 [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning Stefan Hajnoczi
2023-09-12 6:18 ` Philippe Mathieu-Daudé
@ 2023-09-12 6:46 ` Eugenio Perez Martin
2023-09-15 5:30 ` Jason Wang
2 siblings, 0 replies; 6+ messages in thread
From: Eugenio Perez Martin @ 2023-09-12 6:46 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Jason Wang, Michael S . Tsirkin
On Mon, Sep 11, 2023 at 11:54 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> gcc 13.2.1 emits the following warning:
>
> net/vhost-vdpa.c: In function ‘net_vhost_vdpa_init.constprop’:
> net/vhost-vdpa.c:1394:25: error: ‘cvq_isolated’ may be used uninitialized [-Werror=maybe-uninitialized]
> 1394 | s->cvq_isolated = cvq_isolated;
> | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> net/vhost-vdpa.c:1355:9: note: ‘cvq_isolated’ was declared here
> 1355 | int cvq_isolated;
> | ^~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
I think GCC is evaluating the case where (queue_pair_index == 0 &&
!is_datapath), which should be not possible.
If it is useful we can refactor the conditionals so we don't need the
variable at that level of the function. If not,
Acked-by: Eugenio Pérez <eperezma@redhat.com>
Thanks!
> Cc: Eugenio Pérez <eperezma@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> net/vhost-vdpa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 34202ca009..7eaee841aa 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1352,7 +1352,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
> VhostVDPAState *s;
> int ret = 0;
> assert(name);
> - int cvq_isolated;
> + int cvq_isolated = 0;
>
> if (is_datapath) {
> nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning
2023-09-12 6:18 ` Philippe Mathieu-Daudé
@ 2023-09-12 10:48 ` Stefan Hajnoczi
2023-09-12 11:17 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-09-12 10:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Stefan Hajnoczi, qemu-devel, Jason Wang, Eugenio Pérez,
Michael S . Tsirkin
On Tue, 12 Sept 2023 at 02:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 11/9/23 23:54, Stefan Hajnoczi wrote:
> > gcc 13.2.1 emits the following warning:
> >
> > net/vhost-vdpa.c: In function ‘net_vhost_vdpa_init.constprop’:
> > net/vhost-vdpa.c:1394:25: error: ‘cvq_isolated’ may be used uninitialized [-Werror=maybe-uninitialized]
> > 1394 | s->cvq_isolated = cvq_isolated;
> > | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> > net/vhost-vdpa.c:1355:9: note: ‘cvq_isolated’ was declared here
> > 1355 | int cvq_isolated;
> > | ^~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> >
> > Cc: Eugenio Pérez <eperezma@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> > net/vhost-vdpa.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > index 34202ca009..7eaee841aa 100644
> > --- a/net/vhost-vdpa.c
> > +++ b/net/vhost-vdpa.c
> > @@ -1352,7 +1352,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
> > VhostVDPAState *s;
> > int ret = 0;
> > assert(name);
> > - int cvq_isolated;
> > + int cvq_isolated = 0;
> >
> > if (is_datapath) {
> > nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
>
> Alternatively:
>
> -- >8 --
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 34202ca009..218fe0c305 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1352,13 +1352,12 @@ static NetClientState
> *net_vhost_vdpa_init(NetClientState *peer,
> VhostVDPAState *s;
> int ret = 0;
> assert(name);
> - int cvq_isolated;
>
> if (is_datapath) {
> nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
> name);
> } else {
> - cvq_isolated = vhost_vdpa_probe_cvq_isolation(vdpa_device_fd,
> features,
> + int cvq_isolated =
> vhost_vdpa_probe_cvq_isolation(vdpa_device_fd, features,
> queue_pair_index
> * 2,
> errp);
> if (unlikely(cvq_isolated < 0)) {
> @@ -1391,7 +1390,7 @@ static NetClientState
> *net_vhost_vdpa_init(NetClientState *peer,
>
> s->vhost_vdpa.shadow_vq_ops = &vhost_vdpa_net_svq_ops;
> s->vhost_vdpa.shadow_vq_ops_opaque = s;
> - s->cvq_isolated = cvq_isolated;
> + s->cvq_isolated = true;
This is incorrect because the return value of
vhost_vdpa_probe_cvq_isolation() is -errno for errors, 0 for no cvq
isolation, and 1 for cvq isolation. A variable is still needed to
distinguish between no cvq isolation and cvq isolation.
>
> ---
>
> Whichever you prefer:
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning
2023-09-12 10:48 ` Stefan Hajnoczi
@ 2023-09-12 11:17 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-12 11:17 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Stefan Hajnoczi, qemu-devel, Jason Wang, Eugenio Pérez,
Michael S . Tsirkin
On 12/9/23 12:48, Stefan Hajnoczi wrote:
> On Tue, 12 Sept 2023 at 02:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 11/9/23 23:54, Stefan Hajnoczi wrote:
>>> gcc 13.2.1 emits the following warning:
>>>
>>> net/vhost-vdpa.c: In function ‘net_vhost_vdpa_init.constprop’:
>>> net/vhost-vdpa.c:1394:25: error: ‘cvq_isolated’ may be used uninitialized [-Werror=maybe-uninitialized]
>>> 1394 | s->cvq_isolated = cvq_isolated;
>>> | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
>>> net/vhost-vdpa.c:1355:9: note: ‘cvq_isolated’ was declared here
>>> 1355 | int cvq_isolated;
>>> | ^~~~~~~~~~~~
>>> cc1: all warnings being treated as errors
>>>
>>> Cc: Eugenio Pérez <eperezma@redhat.com>
>>> Cc: Michael S. Tsirkin <mst@redhat.com>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>> net/vhost-vdpa.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>>> index 34202ca009..7eaee841aa 100644
>>> --- a/net/vhost-vdpa.c
>>> +++ b/net/vhost-vdpa.c
>>> @@ -1352,7 +1352,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
>>> VhostVDPAState *s;
>>> int ret = 0;
>>> assert(name);
>>> - int cvq_isolated;
>>> + int cvq_isolated = 0;
>>>
>>> if (is_datapath) {
>>> nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
>>
>> Alternatively:
>>
>> -- >8 --
>> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
>> index 34202ca009..218fe0c305 100644
>> --- a/net/vhost-vdpa.c
>> +++ b/net/vhost-vdpa.c
>> @@ -1352,13 +1352,12 @@ static NetClientState
>> *net_vhost_vdpa_init(NetClientState *peer,
>> VhostVDPAState *s;
>> int ret = 0;
>> assert(name);
>> - int cvq_isolated;
>>
>> if (is_datapath) {
>> nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
>> name);
>> } else {
>> - cvq_isolated = vhost_vdpa_probe_cvq_isolation(vdpa_device_fd,
>> features,
>> + int cvq_isolated =
>> vhost_vdpa_probe_cvq_isolation(vdpa_device_fd, features,
>> queue_pair_index
>> * 2,
>> errp);
>> if (unlikely(cvq_isolated < 0)) {
>> @@ -1391,7 +1390,7 @@ static NetClientState
>> *net_vhost_vdpa_init(NetClientState *peer,
>>
>> s->vhost_vdpa.shadow_vq_ops = &vhost_vdpa_net_svq_ops;
>> s->vhost_vdpa.shadow_vq_ops_opaque = s;
>> - s->cvq_isolated = cvq_isolated;
>> + s->cvq_isolated = true;
>
> This is incorrect because the return value of
> vhost_vdpa_probe_cvq_isolation() is -errno for errors, 0 for no cvq
> isolation, and 1 for cvq isolation. A variable is still needed to
> distinguish between no cvq isolation and cvq isolation.
Oh, I was expecting a boolean for a field named 'cvq_isolated',
my bad. R-b stands for your patch.
Regards,
Phil.
>
>>
>> ---
>>
>> Whichever you prefer:
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning
2023-09-11 21:54 [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning Stefan Hajnoczi
2023-09-12 6:18 ` Philippe Mathieu-Daudé
2023-09-12 6:46 ` Eugenio Perez Martin
@ 2023-09-15 5:30 ` Jason Wang
2 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2023-09-15 5:30 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, Eugenio Pérez, Michael S . Tsirkin
On Tue, Sep 12, 2023 at 5:54 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> gcc 13.2.1 emits the following warning:
>
> net/vhost-vdpa.c: In function ‘net_vhost_vdpa_init.constprop’:
> net/vhost-vdpa.c:1394:25: error: ‘cvq_isolated’ may be used uninitialized [-Werror=maybe-uninitialized]
> 1394 | s->cvq_isolated = cvq_isolated;
> | ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> net/vhost-vdpa.c:1355:9: note: ‘cvq_isolated’ was declared here
> 1355 | int cvq_isolated;
> | ^~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Cc: Eugenio Pérez <eperezma@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
> ---
> net/vhost-vdpa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index 34202ca009..7eaee841aa 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -1352,7 +1352,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState *peer,
> VhostVDPAState *s;
> int ret = 0;
> assert(name);
> - int cvq_isolated;
> + int cvq_isolated = 0;
>
> if (is_datapath) {
> nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device,
> --
> 2.41.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-15 5:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-11 21:54 [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning Stefan Hajnoczi
2023-09-12 6:18 ` Philippe Mathieu-Daudé
2023-09-12 10:48 ` Stefan Hajnoczi
2023-09-12 11:17 ` Philippe Mathieu-Daudé
2023-09-12 6:46 ` Eugenio Perez Martin
2023-09-15 5:30 ` Jason Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).