| Message ID | 20190322143643.1317312-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rbd: avoid clang -Wuninitialized warning | expand |
On Fri, Mar 22, 2019 at 3:36 PM Arnd Bergmann <arnd@arndb.de> wrote: > > clang fails to see that rbd_assert(0) ends in an unreachable code > path and warns about a subsequent use of an uninitialized variable > when CONFIG_PROFILE_ANNOTATED_BRANCHES is set: > > drivers/block/rbd.c:2402:4: error: variable 'ret' is used uninitialized whenever 'if' condition is false > [-Werror,-Wsometimes-uninitialized] > rbd_assert(0); > ^~~~~~~~~~~~~ > drivers/block/rbd.c:563:7: note: expanded from macro 'rbd_assert' > if (unlikely(!(expr))) { \ > ^~~~~~~~~~~~~~~~~ > include/linux/compiler.h:48:23: note: expanded from macro 'unlikely' > # define unlikely(x) (__branch_check__(x, 0, __builtin_constant_p(x))) > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/block/rbd.c:2410:6: note: uninitialized use occurs here > if (ret) { > ^~~ > drivers/block/rbd.c:2402:4: note: remove the 'if' if its condition is always true > rbd_assert(0); > ^ > drivers/block/rbd.c:563:3: note: expanded from macro 'rbd_assert' > if (unlikely(!(expr))) { \ > ^ > drivers/block/rbd.c:2376:9: note: initialize the variable 'ret' to silence this warning > int ret; > ^ > = 0 > 1 error generated. > > This seems to be a bug in clang, but is easy to work around by using > an unconditional BUG(). > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/block/rbd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index 4ba967d65cf9..cbcc3baf3807 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -2399,7 +2399,7 @@ static int rbd_obj_read_from_parent(struct rbd_obj_request *obj_req) > &obj_req->bvec_pos); > break; > default: > - rbd_assert(0); > + BUG(); > } > } else { > ret = rbd_img_fill_from_bvecs(child_img_req, Hi Arnd, You did a couple of these last year in commit c6244b3b2377 ("rbd: avoid Wreturn-type warnings"). Let's change all of those default cases to BUG in one go. Do you want to do that or should I? Thanks, Ilya
On Fri, Mar 22, 2019 at 5:33 PM Ilya Dryomov <idryomov@gmail.com> wrote: > > On Fri, Mar 22, 2019 at 3:36 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > > index 4ba967d65cf9..cbcc3baf3807 100644 > > --- a/drivers/block/rbd.c > > +++ b/drivers/block/rbd.c > > @@ -2399,7 +2399,7 @@ static int rbd_obj_read_from_parent(struct rbd_obj_request *obj_req) > > &obj_req->bvec_pos); > > break; > > default: > > - rbd_assert(0); > > + BUG(); > > } > > } else { > > ret = rbd_img_fill_from_bvecs(child_img_req, > > Hi Arnd, > > You did a couple of these last year in commit c6244b3b2377 ("rbd: avoid > Wreturn-type warnings"). Ah, I had completely forgotten about that. Different bug and different compiler, but same solution ;-) > Let's change all of those default cases to BUG > in one go. Do you want to do that or should I? I've prepared another patch now and sent it out, please apply it on top. I'd like this one-line patch to stay separate though since it captures the warning message and may need to be backported to stable kernels later. Arnd
On Fri, Mar 22, 2019 at 5:55 PM Arnd Bergmann <arnd@arndb.de> wrote: > > On Fri, Mar 22, 2019 at 5:33 PM Ilya Dryomov <idryomov@gmail.com> wrote: > > > > On Fri, Mar 22, 2019 at 3:36 PM Arnd Bergmann <arnd@arndb.de> wrote: > > > > > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > > > index 4ba967d65cf9..cbcc3baf3807 100644 > > > --- a/drivers/block/rbd.c > > > +++ b/drivers/block/rbd.c > > > @@ -2399,7 +2399,7 @@ static int rbd_obj_read_from_parent(struct rbd_obj_request *obj_req) > > > &obj_req->bvec_pos); > > > break; > > > default: > > > - rbd_assert(0); > > > + BUG(); > > > } > > > } else { > > > ret = rbd_img_fill_from_bvecs(child_img_req, > > > > Hi Arnd, > > > > You did a couple of these last year in commit c6244b3b2377 ("rbd: avoid > > Wreturn-type warnings"). > > Ah, I had completely forgotten about that. Different bug and different > compiler, but same solution ;-) > > > Let's change all of those default cases to BUG > > in one go. Do you want to do that or should I? > > I've prepared another patch now and sent it out, please > apply it on top. I'd like this one-line patch to stay separate though > since it captures the warning message and may need to > be backported to stable kernels later. Applied. Thanks, Ilya
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 4ba967d65cf9..cbcc3baf3807 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -2399,7 +2399,7 @@ static int rbd_obj_read_from_parent(struct rbd_obj_request *obj_req) &obj_req->bvec_pos); break; default: - rbd_assert(0); + BUG(); } } else { ret = rbd_img_fill_from_bvecs(child_img_req,
clang fails to see that rbd_assert(0) ends in an unreachable code path and warns about a subsequent use of an uninitialized variable when CONFIG_PROFILE_ANNOTATED_BRANCHES is set: drivers/block/rbd.c:2402:4: error: variable 'ret' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized] rbd_assert(0); ^~~~~~~~~~~~~ drivers/block/rbd.c:563:7: note: expanded from macro 'rbd_assert' if (unlikely(!(expr))) { \ ^~~~~~~~~~~~~~~~~ include/linux/compiler.h:48:23: note: expanded from macro 'unlikely' # define unlikely(x) (__branch_check__(x, 0, __builtin_constant_p(x))) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/block/rbd.c:2410:6: note: uninitialized use occurs here if (ret) { ^~~ drivers/block/rbd.c:2402:4: note: remove the 'if' if its condition is always true rbd_assert(0); ^ drivers/block/rbd.c:563:3: note: expanded from macro 'rbd_assert' if (unlikely(!(expr))) { \ ^ drivers/block/rbd.c:2376:9: note: initialize the variable 'ret' to silence this warning int ret; ^ = 0 1 error generated. This seems to be a bug in clang, but is easy to work around by using an unconditional BUG(). Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/block/rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)