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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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