* [PATCH] bcache: avoid clang -Wunintialized warning
@ 2019-03-22 14:35 Arnd Bergmann
2019-03-22 16:03 ` Nathan Chancellor
2019-04-25 18:08 ` Nathan Chancellor
0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:35 UTC (permalink / raw)
To: Coly Li, Kent Overstreet
Cc: clang-built-linux, Nick Desaulniers, Nathan Chancellor,
Arnd Bergmann, Jens Axboe, Shenghui Wang, Hannes Reinecke,
linux-bcache, linux-kernel
clang has identified a code path in which it thinks a
variable may be unused:
drivers/md/bcache/alloc.c:333:4: error: variable 'bucket' is used uninitialized whenever 'if' condition is false
[-Werror,-Wsometimes-uninitialized]
fifo_pop(&ca->free_inc, bucket);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop'
#define fifo_pop(fifo, i) fifo_pop_front(fifo, (i))
^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/md/bcache/util.h:189:6: note: expanded from macro 'fifo_pop_front'
if (_r) { \
^~
drivers/md/bcache/alloc.c:343:46: note: uninitialized use occurs here
allocator_wait(ca, bch_allocator_push(ca, bucket));
^~~~~~
drivers/md/bcache/alloc.c:287:7: note: expanded from macro 'allocator_wait'
if (cond) \
^~~~
drivers/md/bcache/alloc.c:333:4: note: remove the 'if' if its condition is always true
fifo_pop(&ca->free_inc, bucket);
^
drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop'
#define fifo_pop(fifo, i) fifo_pop_front(fifo, (i))
^
drivers/md/bcache/util.h:189:2: note: expanded from macro 'fifo_pop_front'
if (_r) { \
^
drivers/md/bcache/alloc.c:331:15: note: initialize the variable 'bucket' to silence this warning
long bucket;
^
This cannot happen in practice because we only enter the loop
if there is at least one element in the list.
Slightly rearranging the code makes this clearer to both the
reader and the compiler, which avoids the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/md/bcache/alloc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
index 5002838ea476..f8986effcb50 100644
--- a/drivers/md/bcache/alloc.c
+++ b/drivers/md/bcache/alloc.c
@@ -327,10 +327,11 @@ static int bch_allocator_thread(void *arg)
* possibly issue discards to them, then we add the bucket to
* the free list:
*/
- while (!fifo_empty(&ca->free_inc)) {
+ while (1) {
long bucket;
- fifo_pop(&ca->free_inc, bucket);
+ if (!fifo_pop(&ca->free_inc, bucket))
+ break;
if (ca->discard) {
mutex_unlock(&ca->set->bucket_lock);
--
2.20.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] bcache: avoid clang -Wunintialized warning 2019-03-22 14:35 [PATCH] bcache: avoid clang -Wunintialized warning Arnd Bergmann @ 2019-03-22 16:03 ` Nathan Chancellor 2019-04-25 18:08 ` Nathan Chancellor 1 sibling, 0 replies; 6+ messages in thread From: Nathan Chancellor @ 2019-03-22 16:03 UTC (permalink / raw) To: Arnd Bergmann Cc: Coly Li, Kent Overstreet, clang-built-linux, Nick Desaulniers, Jens Axboe, Shenghui Wang, Hannes Reinecke, linux-bcache, linux-kernel On Fri, Mar 22, 2019 at 03:35:00PM +0100, Arnd Bergmann wrote: > clang has identified a code path in which it thinks a > variable may be unused: > > drivers/md/bcache/alloc.c:333:4: error: variable 'bucket' is used uninitialized whenever 'if' condition is false > [-Werror,-Wsometimes-uninitialized] > fifo_pop(&ca->free_inc, bucket); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' > #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) > ^~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/md/bcache/util.h:189:6: note: expanded from macro 'fifo_pop_front' > if (_r) { \ > ^~ > drivers/md/bcache/alloc.c:343:46: note: uninitialized use occurs here > allocator_wait(ca, bch_allocator_push(ca, bucket)); > ^~~~~~ > drivers/md/bcache/alloc.c:287:7: note: expanded from macro 'allocator_wait' > if (cond) \ > ^~~~ > drivers/md/bcache/alloc.c:333:4: note: remove the 'if' if its condition is always true > fifo_pop(&ca->free_inc, bucket); > ^ > drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' > #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) > ^ > drivers/md/bcache/util.h:189:2: note: expanded from macro 'fifo_pop_front' > if (_r) { \ > ^ > drivers/md/bcache/alloc.c:331:15: note: initialize the variable 'bucket' to silence this warning > long bucket; > ^ > > This cannot happen in practice because we only enter the loop > if there is at least one element in the list. > > Slightly rearranging the code makes this clearer to both the > reader and the compiler, which avoids the warning. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Yes, I like this much better than my patch, thanks! Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/md/bcache/alloc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c > index 5002838ea476..f8986effcb50 100644 > --- a/drivers/md/bcache/alloc.c > +++ b/drivers/md/bcache/alloc.c > @@ -327,10 +327,11 @@ static int bch_allocator_thread(void *arg) > * possibly issue discards to them, then we add the bucket to > * the free list: > */ > - while (!fifo_empty(&ca->free_inc)) { > + while (1) { > long bucket; > > - fifo_pop(&ca->free_inc, bucket); > + if (!fifo_pop(&ca->free_inc, bucket)) > + break; > > if (ca->discard) { > mutex_unlock(&ca->set->bucket_lock); > -- > 2.20.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: avoid clang -Wunintialized warning 2019-03-22 14:35 [PATCH] bcache: avoid clang -Wunintialized warning Arnd Bergmann 2019-03-22 16:03 ` Nathan Chancellor @ 2019-04-25 18:08 ` Nathan Chancellor 2019-04-26 2:43 ` Coly Li 1 sibling, 1 reply; 6+ messages in thread From: Nathan Chancellor @ 2019-04-25 18:08 UTC (permalink / raw) To: Arnd Bergmann Cc: Coly Li, Kent Overstreet, clang-built-linux, Nick Desaulniers, Jens Axboe, Shenghui Wang, Hannes Reinecke, linux-bcache, linux-kernel On Fri, Mar 22, 2019 at 03:35:00PM +0100, Arnd Bergmann wrote: > clang has identified a code path in which it thinks a > variable may be unused: > > drivers/md/bcache/alloc.c:333:4: error: variable 'bucket' is used uninitialized whenever 'if' condition is false > [-Werror,-Wsometimes-uninitialized] > fifo_pop(&ca->free_inc, bucket); > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' > #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) > ^~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/md/bcache/util.h:189:6: note: expanded from macro 'fifo_pop_front' > if (_r) { \ > ^~ > drivers/md/bcache/alloc.c:343:46: note: uninitialized use occurs here > allocator_wait(ca, bch_allocator_push(ca, bucket)); > ^~~~~~ > drivers/md/bcache/alloc.c:287:7: note: expanded from macro 'allocator_wait' > if (cond) \ > ^~~~ > drivers/md/bcache/alloc.c:333:4: note: remove the 'if' if its condition is always true > fifo_pop(&ca->free_inc, bucket); > ^ > drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' > #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) > ^ > drivers/md/bcache/util.h:189:2: note: expanded from macro 'fifo_pop_front' > if (_r) { \ > ^ > drivers/md/bcache/alloc.c:331:15: note: initialize the variable 'bucket' to silence this warning > long bucket; > ^ > > This cannot happen in practice because we only enter the loop > if there is at least one element in the list. > > Slightly rearranging the code makes this clearer to both the > reader and the compiler, which avoids the warning. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/md/bcache/alloc.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c > index 5002838ea476..f8986effcb50 100644 > --- a/drivers/md/bcache/alloc.c > +++ b/drivers/md/bcache/alloc.c > @@ -327,10 +327,11 @@ static int bch_allocator_thread(void *arg) > * possibly issue discards to them, then we add the bucket to > * the free list: > */ > - while (!fifo_empty(&ca->free_inc)) { > + while (1) { > long bucket; > > - fifo_pop(&ca->free_inc, bucket); > + if (!fifo_pop(&ca->free_inc, bucket)) > + break; > > if (ca->discard) { > mutex_unlock(&ca->set->bucket_lock); > -- > 2.20.0 > Hi all, Could someone please review/pick this up? This is one of two remaining -Wsometimes-uninitialized warnings among arm, arm64, and x86_64 all{yes,mod}config and I'd like to get it turned on as soon as possible to catch more bugs. Thanks, Nathan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: avoid clang -Wunintialized warning 2019-04-25 18:08 ` Nathan Chancellor @ 2019-04-26 2:43 ` Coly Li 2019-04-26 13:01 ` Nathan Chancellor 0 siblings, 1 reply; 6+ messages in thread From: Coly Li @ 2019-04-26 2:43 UTC (permalink / raw) To: Nathan Chancellor, Arnd Bergmann Cc: Kent Overstreet, clang-built-linux, Nick Desaulniers, Jens Axboe, Shenghui Wang, Hannes Reinecke, linux-bcache, linux-kernel On 2019/4/26 2:08 上午, Nathan Chancellor wrote: > On Fri, Mar 22, 2019 at 03:35:00PM +0100, Arnd Bergmann wrote: >> clang has identified a code path in which it thinks a >> variable may be unused: >> >> drivers/md/bcache/alloc.c:333:4: error: variable 'bucket' is used uninitialized whenever 'if' condition is false >> [-Werror,-Wsometimes-uninitialized] >> fifo_pop(&ca->free_inc, bucket); >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' >> #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) >> ^~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/md/bcache/util.h:189:6: note: expanded from macro 'fifo_pop_front' >> if (_r) { \ >> ^~ >> drivers/md/bcache/alloc.c:343:46: note: uninitialized use occurs here >> allocator_wait(ca, bch_allocator_push(ca, bucket)); >> ^~~~~~ >> drivers/md/bcache/alloc.c:287:7: note: expanded from macro 'allocator_wait' >> if (cond) \ >> ^~~~ >> drivers/md/bcache/alloc.c:333:4: note: remove the 'if' if its condition is always true >> fifo_pop(&ca->free_inc, bucket); >> ^ >> drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' >> #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) >> ^ >> drivers/md/bcache/util.h:189:2: note: expanded from macro 'fifo_pop_front' >> if (_r) { \ >> ^ >> drivers/md/bcache/alloc.c:331:15: note: initialize the variable 'bucket' to silence this warning >> long bucket; >> ^ >> >> This cannot happen in practice because we only enter the loop >> if there is at least one element in the list. >> >> Slightly rearranging the code makes this clearer to both the >> reader and the compiler, which avoids the warning. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> drivers/md/bcache/alloc.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c >> index 5002838ea476..f8986effcb50 100644 >> --- a/drivers/md/bcache/alloc.c >> +++ b/drivers/md/bcache/alloc.c >> @@ -327,10 +327,11 @@ static int bch_allocator_thread(void *arg) >> * possibly issue discards to them, then we add the bucket to >> * the free list: >> */ >> - while (!fifo_empty(&ca->free_inc)) { >> + while (1) { >> long bucket; >> >> - fifo_pop(&ca->free_inc, bucket); >> + if (!fifo_pop(&ca->free_inc, bucket)) >> + break; >> >> if (ca->discard) { >> mutex_unlock(&ca->set->bucket_lock); >> -- >> 2.20.0 >> > > Hi all, > > Could someone please review/pick this up? This is one of two remaining > -Wsometimes-uninitialized warnings among arm, arm64, and x86_64 > all{yes,mod}config and I'd like to get it turned on as soon as possible > to catch more bugs. Hi Nathan, It is in Jens' block tree for-next branch already, for Linux v5.2 merge window. Thanks. -- Coly Li ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: avoid clang -Wunintialized warning 2019-04-26 2:43 ` Coly Li @ 2019-04-26 13:01 ` Nathan Chancellor 2019-04-26 14:27 ` Coly Li 0 siblings, 1 reply; 6+ messages in thread From: Nathan Chancellor @ 2019-04-26 13:01 UTC (permalink / raw) To: Coly Li Cc: Arnd Bergmann, Kent Overstreet, clang-built-linux, Nick Desaulniers, Jens Axboe, Shenghui Wang, Hannes Reinecke, linux-bcache, linux-kernel On Fri, Apr 26, 2019 at 10:43:01AM +0800, Coly Li wrote: > On 2019/4/26 2:08 上午, Nathan Chancellor wrote: > > On Fri, Mar 22, 2019 at 03:35:00PM +0100, Arnd Bergmann wrote: > >> clang has identified a code path in which it thinks a > >> variable may be unused: > >> > >> drivers/md/bcache/alloc.c:333:4: error: variable 'bucket' is used uninitialized whenever 'if' condition is false > >> [-Werror,-Wsometimes-uninitialized] > >> fifo_pop(&ca->free_inc, bucket); > >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >> drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' > >> #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) > >> ^~~~~~~~~~~~~~~~~~~~~~~~~ > >> drivers/md/bcache/util.h:189:6: note: expanded from macro 'fifo_pop_front' > >> if (_r) { \ > >> ^~ > >> drivers/md/bcache/alloc.c:343:46: note: uninitialized use occurs here > >> allocator_wait(ca, bch_allocator_push(ca, bucket)); > >> ^~~~~~ > >> drivers/md/bcache/alloc.c:287:7: note: expanded from macro 'allocator_wait' > >> if (cond) \ > >> ^~~~ > >> drivers/md/bcache/alloc.c:333:4: note: remove the 'if' if its condition is always true > >> fifo_pop(&ca->free_inc, bucket); > >> ^ > >> drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' > >> #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) > >> ^ > >> drivers/md/bcache/util.h:189:2: note: expanded from macro 'fifo_pop_front' > >> if (_r) { \ > >> ^ > >> drivers/md/bcache/alloc.c:331:15: note: initialize the variable 'bucket' to silence this warning > >> long bucket; > >> ^ > >> > >> This cannot happen in practice because we only enter the loop > >> if there is at least one element in the list. > >> > >> Slightly rearranging the code makes this clearer to both the > >> reader and the compiler, which avoids the warning. > >> > >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > >> --- > >> drivers/md/bcache/alloc.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c > >> index 5002838ea476..f8986effcb50 100644 > >> --- a/drivers/md/bcache/alloc.c > >> +++ b/drivers/md/bcache/alloc.c > >> @@ -327,10 +327,11 @@ static int bch_allocator_thread(void *arg) > >> * possibly issue discards to them, then we add the bucket to > >> * the free list: > >> */ > >> - while (!fifo_empty(&ca->free_inc)) { > >> + while (1) { > >> long bucket; > >> > >> - fifo_pop(&ca->free_inc, bucket); > >> + if (!fifo_pop(&ca->free_inc, bucket)) > >> + break; > >> > >> if (ca->discard) { > >> mutex_unlock(&ca->set->bucket_lock); > >> -- > >> 2.20.0 > >> > > > > Hi all, > > > > Could someone please review/pick this up? This is one of two remaining > > -Wsometimes-uninitialized warnings among arm, arm64, and x86_64 > > all{yes,mod}config and I'd like to get it turned on as soon as possible > > to catch more bugs. > > Hi Nathan, > > It is in Jens' block tree for-next branch already, for Linux v5.2 merge > window. > > Thanks. > > -- > > Coly Li Hi Coly, Thank you for the reply and heads up, it hadn't hit -next when I sent that message and I didn't check Jens' tree. I appreciate you picking it up! Nathan ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcache: avoid clang -Wunintialized warning 2019-04-26 13:01 ` Nathan Chancellor @ 2019-04-26 14:27 ` Coly Li 0 siblings, 0 replies; 6+ messages in thread From: Coly Li @ 2019-04-26 14:27 UTC (permalink / raw) To: Nathan Chancellor Cc: Arnd Bergmann, Kent Overstreet, clang-built-linux, Nick Desaulniers, Jens Axboe, Shenghui Wang, Hannes Reinecke, linux-bcache, linux-kernel On 2019/4/26 9:01 下午, Nathan Chancellor wrote: > On Fri, Apr 26, 2019 at 10:43:01AM +0800, Coly Li wrote: >> On 2019/4/26 2:08 上午, Nathan Chancellor wrote: >>> On Fri, Mar 22, 2019 at 03:35:00PM +0100, Arnd Bergmann wrote: >>>> clang has identified a code path in which it thinks a >>>> variable may be unused: >>>> >>>> drivers/md/bcache/alloc.c:333:4: error: variable 'bucket' is used uninitialized whenever 'if' condition is false >>>> [-Werror,-Wsometimes-uninitialized] >>>> fifo_pop(&ca->free_inc, bucket); >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' >>>> #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~ >>>> drivers/md/bcache/util.h:189:6: note: expanded from macro 'fifo_pop_front' >>>> if (_r) { \ >>>> ^~ >>>> drivers/md/bcache/alloc.c:343:46: note: uninitialized use occurs here >>>> allocator_wait(ca, bch_allocator_push(ca, bucket)); >>>> ^~~~~~ >>>> drivers/md/bcache/alloc.c:287:7: note: expanded from macro 'allocator_wait' >>>> if (cond) \ >>>> ^~~~ >>>> drivers/md/bcache/alloc.c:333:4: note: remove the 'if' if its condition is always true >>>> fifo_pop(&ca->free_inc, bucket); >>>> ^ >>>> drivers/md/bcache/util.h:219:27: note: expanded from macro 'fifo_pop' >>>> #define fifo_pop(fifo, i) fifo_pop_front(fifo, (i)) >>>> ^ >>>> drivers/md/bcache/util.h:189:2: note: expanded from macro 'fifo_pop_front' >>>> if (_r) { \ >>>> ^ >>>> drivers/md/bcache/alloc.c:331:15: note: initialize the variable 'bucket' to silence this warning >>>> long bucket; >>>> ^ >>>> >>>> This cannot happen in practice because we only enter the loop >>>> if there is at least one element in the list. >>>> >>>> Slightly rearranging the code makes this clearer to both the >>>> reader and the compiler, which avoids the warning. >>>> >>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>>> --- >>>> drivers/md/bcache/alloc.c | 5 +++-- >>>> 1 file changed, 3 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c >>>> index 5002838ea476..f8986effcb50 100644 >>>> --- a/drivers/md/bcache/alloc.c >>>> +++ b/drivers/md/bcache/alloc.c >>>> @@ -327,10 +327,11 @@ static int bch_allocator_thread(void *arg) >>>> * possibly issue discards to them, then we add the bucket to >>>> * the free list: >>>> */ >>>> - while (!fifo_empty(&ca->free_inc)) { >>>> + while (1) { >>>> long bucket; >>>> >>>> - fifo_pop(&ca->free_inc, bucket); >>>> + if (!fifo_pop(&ca->free_inc, bucket)) >>>> + break; >>>> >>>> if (ca->discard) { >>>> mutex_unlock(&ca->set->bucket_lock); >>>> -- >>>> 2.20.0 >>>> >>> >>> Hi all, >>> >>> Could someone please review/pick this up? This is one of two remaining >>> -Wsometimes-uninitialized warnings among arm, arm64, and x86_64 >>> all{yes,mod}config and I'd like to get it turned on as soon as possible >>> to catch more bugs. >> >> Hi Nathan, >> >> It is in Jens' block tree for-next branch already, for Linux v5.2 merge >> window. >> >> Thanks. >> >> -- >> >> Coly Li > > Hi Coly, > > Thank you for the reply and heads up, it hadn't hit -next when I sent > that message and I didn't check Jens' tree. > > I appreciate you picking it up! Hi Nathan, You are welcome, and thanks for the contribution to make bcache better :-) -- Coly Li ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-26 14:27 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-22 14:35 [PATCH] bcache: avoid clang -Wunintialized warning Arnd Bergmann 2019-03-22 16:03 ` Nathan Chancellor 2019-04-25 18:08 ` Nathan Chancellor 2019-04-26 2:43 ` Coly Li 2019-04-26 13:01 ` Nathan Chancellor 2019-04-26 14:27 ` Coly Li
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).