Re: [PATCH] vdpa: fix gcc cvq_isolated uninitialized variable warning

[Stefan Hajnoczi]

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>
>
>