* [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning
@ 2020-08-18 4:03 Brooke Basile
2020-08-18 5:29 ` Nathan Chancellor
0 siblings, 1 reply; 7+ messages in thread
From: Brooke Basile @ 2020-08-18 4:03 UTC (permalink / raw)
To: danil.kipnis, jinpu.wang, axboe
Cc: linux-block, linux-kernel, clang-built-linux, Brooke Basile
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@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning 2020-08-18 4:03 [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning Brooke Basile @ 2020-08-18 5:29 ` Nathan Chancellor 2020-08-18 6:42 ` Jinpu Wang 2020-08-18 12:43 ` [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning Brooke Basile 0 siblings, 2 replies; 7+ messages in thread From: Nathan Chancellor @ 2020-08-18 5:29 UTC (permalink / raw) To: Brooke Basile Cc: danil.kipnis, jinpu.wang, axboe, linux-block, linux-kernel, clang-built-linux 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@gmail.com> > --- > 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; } ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning 2020-08-18 5:29 ` Nathan Chancellor @ 2020-08-18 6:42 ` Jinpu Wang 2020-08-18 6:49 ` [PATCH] block/rnbd: Ensure err is always initialized in process_rdma Nathan Chancellor 2020-08-18 12:43 ` [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning Brooke Basile 1 sibling, 1 reply; 7+ messages in thread From: Jinpu Wang @ 2020-08-18 6:42 UTC (permalink / raw) To: Nathan Chancellor Cc: Brooke Basile, Danil Kipnis, Jens Axboe, linux-block, linux-kernel, clang-built-linux On Tue, Aug 18, 2020 at 7:30 AM Nathan Chancellor <natechancellor@gmail.com> 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 <brookebasile@gmail.com> > > --- > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] block/rnbd: Ensure err is always initialized in process_rdma 2020-08-18 6:42 ` Jinpu Wang @ 2020-08-18 6:49 ` Nathan Chancellor 2020-08-18 6:55 ` Jinpu Wang 2020-08-18 14:49 ` Jens Axboe 0 siblings, 2 replies; 7+ messages in thread From: Nathan Chancellor @ 2020-08-18 6:49 UTC (permalink / raw) To: Danil Kipnis, Jack Wang Cc: Jens Axboe, Guoqing Jiang, linux-block, linux-kernel, clang-built-linux, Nathan Chancellor, Brooke Basile 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:150:2: note: remove the 'if' if its condition is always false if (IS_ERR(bio)) { ^~~~~~~~~~~~~~~~~~ drivers/block/rnbd/rnbd-srv.c:126:9: note: initialize the variable 'err' to silence this warning int err; ^ = 0 1 warning generated. err is indeed uninitialized when this statement is taken. Ensure that it is assigned the error value of bio before jumping to the error handling label. Fixes: 735d77d4fd28 ("rnbd: remove rnbd_dev_submit_io") Link: https://github.com/ClangBuiltLinux/linux/issues/1134 Reported-by: Brooke Basile <brookebasile@gmail.com> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/block/rnbd/rnbd-srv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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; } -- 2.28.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] block/rnbd: Ensure err is always initialized in process_rdma 2020-08-18 6:49 ` [PATCH] block/rnbd: Ensure err is always initialized in process_rdma Nathan Chancellor @ 2020-08-18 6:55 ` Jinpu Wang 2020-08-18 14:49 ` Jens Axboe 1 sibling, 0 replies; 7+ messages in thread From: Jinpu Wang @ 2020-08-18 6:55 UTC (permalink / raw) To: Nathan Chancellor Cc: Danil Kipnis, Jens Axboe, Guoqing Jiang, linux-block, linux-kernel, clang-built-linux, Brooke Basile On Tue, Aug 18, 2020 at 8:50 AM Nathan Chancellor <natechancellor@gmail.com> 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:150:2: note: remove the 'if' if its > condition is always false > if (IS_ERR(bio)) { > ^~~~~~~~~~~~~~~~~~ > drivers/block/rnbd/rnbd-srv.c:126:9: note: initialize the variable 'err' > to silence this warning > int err; > ^ > = 0 > 1 warning generated. > > err is indeed uninitialized when this statement is taken. Ensure that it > is assigned the error value of bio before jumping to the error handling > label. > > Fixes: 735d77d4fd28 ("rnbd: remove rnbd_dev_submit_io") > Link: https://github.com/ClangBuiltLinux/linux/issues/1134 > Reported-by: Brooke Basile <brookebasile@gmail.com> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Thanks! Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com> > --- > drivers/block/rnbd/rnbd-srv.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > 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; > } > > -- > 2.28.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] block/rnbd: Ensure err is always initialized in process_rdma 2020-08-18 6:49 ` [PATCH] block/rnbd: Ensure err is always initialized in process_rdma Nathan Chancellor 2020-08-18 6:55 ` Jinpu Wang @ 2020-08-18 14:49 ` Jens Axboe 1 sibling, 0 replies; 7+ messages in thread From: Jens Axboe @ 2020-08-18 14:49 UTC (permalink / raw) To: Nathan Chancellor, Danil Kipnis, Jack Wang Cc: Guoqing Jiang, linux-block, linux-kernel, clang-built-linux, Brooke Basile On 8/17/20 11:49 PM, Nathan Chancellor 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:150:2: note: remove the 'if' if its > condition is always false > if (IS_ERR(bio)) { > ^~~~~~~~~~~~~~~~~~ > drivers/block/rnbd/rnbd-srv.c:126:9: note: initialize the variable 'err' > to silence this warning > int err; > ^ > = 0 > 1 warning generated. > > err is indeed uninitialized when this statement is taken. Ensure that it > is assigned the error value of bio before jumping to the error handling > label. Applied, thanks. -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning 2020-08-18 5:29 ` Nathan Chancellor 2020-08-18 6:42 ` Jinpu Wang @ 2020-08-18 12:43 ` Brooke Basile 1 sibling, 0 replies; 7+ messages in thread From: Brooke Basile @ 2020-08-18 12:43 UTC (permalink / raw) To: Nathan Chancellor Cc: danil.kipnis, jinpu.wang, axboe, linux-block, linux-kernel, clang-built-linux 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-08-18 14:49 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-18 4:03 [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning Brooke Basile 2020-08-18 5:29 ` Nathan Chancellor 2020-08-18 6:42 ` Jinpu Wang 2020-08-18 6:49 ` [PATCH] block/rnbd: Ensure err is always initialized in process_rdma Nathan Chancellor 2020-08-18 6:55 ` Jinpu Wang 2020-08-18 14:49 ` Jens Axboe 2020-08-18 12:43 ` [PATCH] block: rnbd: rnbd-srv: silence uninitialized variable warning Brooke Basile
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).