Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning
On 8/18/20 1:29 AM, Nathan Chancellor wrote: 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; } Ah, I see what you mean. Thanks for the fix! Best, Brooke Basile
Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning
On Tue, Aug 18, 2020 at 7:30 AM Nathan Chancellor wrote: > > 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 > > --- > > 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 Thanks Nathan, thanks Brooke, I agree with Nathan, the problem is we forgot to set the err before "goto sess_dev_put". Nathan's patch is simpler, less code of change. > > 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; > } > Nathan, could you send a formal patch? Thanks
Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning
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 > --- > 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; }
[PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning
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 --- 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