[PATCH] nbd: fix uninitialized variable warning

[PATCH] nbd: fix uninitialized variable warning

[pannengyuan]

From: Pan Nengyuan <pannengyuan@huawei.com>

Fixes:
/mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
/mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
     int ret;

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
 nbd/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nbd/server.c b/nbd/server.c
index 24ebc1a805..7eb3de0842 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2310,7 +2310,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
                                            NBDRequest *request,
                                            uint8_t *data, Error **errp)
 {
-    int ret;
+    int ret = 0;
     int flags;
     NBDExport *exp = client->exp;
     char *msg;
-- 
2.21.0.windows.1

Re: [PATCH] nbd: fix uninitialized variable warning

[Eric Blake]

On 1/5/20 7:54 PM, pannengyuan@huawei.com wrote:
> From: Pan Nengyuan <pannengyuan@huawei.com>
> 
> Fixes:
> /mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
> /mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>       int ret;
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>

False positive in the robot - I cannot see any path where ret is used 
uninitialized.  Closest might be the handling of NBD_CMD_BLOCK_STATUS, 
which looks like:

if (a || b) {
   if (a) {
     ret = ...;
     if (ret < 0) {
       return ret;
     }
   }
   if (b) {
     ret = ...;
     if (ret < 0) {
       return ret;
     }
   }
   return ret;
}

In fact, those 'if (ret < 0)' tests are pointless, since nothing else 
really happens before the final return ret.

If I'm right about this being what trips up the robot, does changing 'if 
(b)' into 'else' solve the problem, rather than adding an initializer? 
And if so, can we clean up the pointless code while at it?

> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
>   nbd/server.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index 24ebc1a805..7eb3de0842 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2310,7 +2310,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
>                                              NBDRequest *request,
>                                              uint8_t *data, Error **errp)
>   {
> -    int ret;
> +    int ret = 0;
>       int flags;
>       NBDExport *exp = client->exp;
>       char *msg;
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] nbd: fix uninitialized variable warning

[Pan Nengyuan]

On 1/8/2020 6:24 AM, Eric Blake wrote:
> On 1/5/20 7:54 PM, pannengyuan@huawei.com wrote:
>> From: Pan Nengyuan <pannengyuan@huawei.com>
>>
>> Fixes:
>> /mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
>> /mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>       int ret;
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
> 
> False positive in the robot - I cannot see any path where ret is used uninitialized.  Closest might be the handling of NBD_CMD_BLOCK_STATUS, which looks like:
> 
> if (a || b) {
>   if (a) {
>     ret = ...;
>     if (ret < 0) {
>       return ret;
>     }
>   }
>   if (b) {
>     ret = ...;
>     if (ret < 0) {
>       return ret;
>     }
>   }
>   return ret;
> }
> 
> In fact, those 'if (ret < 0)' tests are pointless, since nothing else really happens before the final return ret.
> 
> If I'm right about this being what trips up the robot, does changing 'if (b)' into 'else' solve the problem, rather than adding an initializer? And if so, can we clean up the pointless code while at it?

Yes, you are right, Changing 'if(b)' to 'else' solves the problem.
I will change it and clean up the pointless code in next version.

Thanks.

> 
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
>>   nbd/server.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/nbd/server.c b/nbd/server.c
>> index 24ebc1a805..7eb3de0842 100644
>> --- a/nbd/server.c
>> +++ b/nbd/server.c
>> @@ -2310,7 +2310,7 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
>>                                              NBDRequest *request,
>>                                              uint8_t *data, Error **errp)
>>   {
>> -    int ret;
>> +    int ret = 0;
>>       int flags;
>>       NBDExport *exp = client->exp;
>>       char *msg;
>>
>