Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning

From: Nathan Chancellor
Date: Tue Aug 18 2020 - 01:30:17 EST


On Tue, Aug 18, 2020 at 12:03:18AM -0400, Brooke Basile wrote:
> Clang warns:
> drivers/block/rnbd/rnbd-srv.c:150:6: warning: variable 'err' is used
> uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> if (IS_ERR(bio)) {
> ^~~~~~~~~~~
> drivers/block/rnbd/rnbd-srv.c:177:9: note: uninitialized use occurs here
> return err;
> ^~~
> drivers/block/rnbd/rnbd-srv.c:126:9: note: initialize the variable 'err'
> to silence this warning
> int err;
> ^
> = 0
>
> Silence this by replacing `err` with `ret`, returning ret = 0 upon
> success.
>
> Signed-off-by: Brooke Basile <brookebasile@xxxxxxxxx>
> ---
> drivers/block/rnbd/rnbd-srv.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
> index 0fb94843a495..f515d1a048a9 100644
> --- a/drivers/block/rnbd/rnbd-srv.c
> +++ b/drivers/block/rnbd/rnbd-srv.c
> @@ -123,10 +123,10 @@ static int process_rdma(struct rtrs_srv *sess,
> struct rnbd_io_private *priv;
> struct rnbd_srv_sess_dev *sess_dev;
> u32 dev_id;
> - int err;
> struct rnbd_dev_blk_io *io;
> struct bio *bio;
> short prio;
> + int ret = 0;
>
> priv = kmalloc(sizeof(*priv), GFP_KERNEL);
> if (!priv)
> @@ -138,7 +138,7 @@ static int process_rdma(struct rtrs_srv *sess,
> if (IS_ERR(sess_dev)) {
> pr_err_ratelimited("Got I/O request on session %s for unknown device id %d\n",
> srv_sess->sessname, dev_id);
> - err = -ENOTCONN;
> + ret = -ENOTCONN;
> goto err;
> }
>
> @@ -168,13 +168,13 @@ static int process_rdma(struct rtrs_srv *sess,
>
> submit_bio(bio);
>
> - return 0;
> + return ret;
>
> sess_dev_put:
> rnbd_put_sess_dev(sess_dev);
> err:
> kfree(priv);
> - return err;
> + return ret;
> }
>
> static void destroy_device(struct rnbd_srv_dev *dev)
> --
> 2.28.0
>

I don't think this is a proper fix since the root cause of the warning
appears to be that we are ignoring the return value of
rnbd_bio_map_kern. Should we not set err to that value like this
(completely untested)?

Cheers,
Nathan

diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c
index 0fb94843a495..1b71cb2a885d 100644
--- a/drivers/block/rnbd/rnbd-srv.c
+++ b/drivers/block/rnbd/rnbd-srv.c
@@ -148,7 +148,8 @@ static int process_rdma(struct rtrs_srv *sess,
/* Generate bio with pages pointing to the rdma buffer */
bio = rnbd_bio_map_kern(data, sess_dev->rnbd_dev->ibd_bio_set, datalen, GFP_KERNEL);
if (IS_ERR(bio)) {
- rnbd_srv_err(sess_dev, "Failed to generate bio, err: %ld\n", PTR_ERR(bio));
+ err = PTR_ERR(bio);
+ rnbd_srv_err(sess_dev, "Failed to generate bio, err: %ld\n", err);
goto sess_dev_put;
}