diff mbox series

[PULL,3/5] nbd: fix uninitialized variable warning

Message ID 20200108160233.991134-4-laurent@vivier.eu
State New
Headers show
Series [PULL,1/5] MAINTAINERS: Update Yuval Shaia's email address | expand

Commit Message

Laurent Vivier Jan. 8, 2020, 4:02 p.m. UTC
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20200108025132.46956-1-pannengyuan@huawei.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 nbd/server.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

Comments

Eric Blake Feb. 6, 2020, 4:55 p.m. UTC | #1
On 1/8/20 10:02 AM, Laurent Vivier 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>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Message-Id: <20200108025132.46956-1-pannengyuan@huawei.com>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   nbd/server.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
> 

This patch is broken; I first noticed it when testing libnbd, where the 
symptoms are a hung interop/dirty-bitmap.sh test.

> diff --git a/nbd/server.c b/nbd/server.c
> index 24ebc1a80571..87fcd2e7bfac 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2384,20 +2384,12 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
>                                                  !client->export_meta.bitmap,
>                                                  NBD_META_ID_BASE_ALLOCATION,
>                                                  errp);
> -                if (ret < 0) {
> -                    return ret;
> -                }
> -            }
> -
> -            if (client->export_meta.bitmap) {
> +            } else {              /* client->export_meta.bitmap */
>                   ret = nbd_co_send_bitmap(client, request->handle,
>                                            client->exp->export_bitmap,
>                                            request->from, request->len,
>                                            dont_fragment,
>                                            true, NBD_META_ID_DIRTY_BITMAP, errp);
> -                if (ret < 0) {
> -                    return ret;
> -                }
>               }

The NBD spec says that clients can request more than one meta context at 
a time. Qemu does not (and hence we didn't notice the breakage until 
now), but libnbd does, and this change prevents qemu from obeying the 
spec (if the client requested both contexts, then we must return both, 
and not short-circuit after the first).

I'll post a patch to fix this unintended regression soon.
diff mbox series

Patch

diff --git a/nbd/server.c b/nbd/server.c
index 24ebc1a80571..87fcd2e7bfac 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2384,20 +2384,12 @@  static coroutine_fn int nbd_handle_request(NBDClient *client,
                                                !client->export_meta.bitmap,
                                                NBD_META_ID_BASE_ALLOCATION,
                                                errp);
-                if (ret < 0) {
-                    return ret;
-                }
-            }
-
-            if (client->export_meta.bitmap) {
+            } else {              /* client->export_meta.bitmap */
                 ret = nbd_co_send_bitmap(client, request->handle,
                                          client->exp->export_bitmap,
                                          request->from, request->len,
                                          dont_fragment,
                                          true, NBD_META_ID_DIRTY_BITMAP, errp);
-                if (ret < 0) {
-                    return ret;
-                }
             }
 
             return ret;