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

[Philippe Mathieu-Daudé]

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>