2019-03-07 22:13:22

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] bcache: Zero initialize bucket in bch_allocator_thread

When building with -Wsometimes-uninitialized, Clang warns:

drivers/md/bcache/alloc.c:333:4: warning: variable 'bucket' is used
uninitialized whenever 'if' condition is false
[-Wsometimes-uninitialized]

fifo_pop expands to a conditional depending on '!fifo_empty(...)', which
is the same condition as the while loop. It seems Clang can't piece
together that this condition will never been false as long as we are in
the while loop because they are the same condition. It's easy enough to
silence this warning by just explicitly zero initializing bucket.

Link: https://github.com/ClangBuiltLinux/linux/issues/396
Signed-off-by: Nathan Chancellor <[email protected]>
---
drivers/md/bcache/alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
index 5002838ea476..c72094901ac6 100644
--- a/drivers/md/bcache/alloc.c
+++ b/drivers/md/bcache/alloc.c
@@ -328,7 +328,7 @@ static int bch_allocator_thread(void *arg)
* the free list:
*/
while (!fifo_empty(&ca->free_inc)) {
- long bucket;
+ long bucket = 0;

fifo_pop(&ca->free_inc, bucket);

--
2.21.0



2019-03-08 02:11:05

by Coly Li

[permalink] [raw]
Subject: Re: [PATCH] bcache: Zero initialize bucket in bch_allocator_thread

On 2019/3/8 6:10 上午, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/md/bcache/alloc.c:333:4: warning: variable 'bucket' is used
> uninitialized whenever 'if' condition is false
> [-Wsometimes-uninitialized]
>
> fifo_pop expands to a conditional depending on '!fifo_empty(...)', which
> is the same condition as the while loop. It seems Clang can't piece
> together that this condition will never been false as long as we are in
> the while loop because they are the same condition. It's easy enough to
> silence this warning by just explicitly zero initializing bucket.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/396
> Signed-off-by: Nathan Chancellor <[email protected]>

Hi Nathan,

Nice catch, I will add it to my for-test. Thanks.

Coly Li

> ---
> drivers/md/bcache/alloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/bcache/alloc.c b/drivers/md/bcache/alloc.c
> index 5002838ea476..c72094901ac6 100644
> --- a/drivers/md/bcache/alloc.c
> +++ b/drivers/md/bcache/alloc.c
> @@ -328,7 +328,7 @@ static int bch_allocator_thread(void *arg)
> * the free list:
> */
> while (!fifo_empty(&ca->free_inc)) {
> - long bucket;
> + long bucket = 0;
>
> fifo_pop(&ca->free_inc, bucket);
>
>


--

Coly Li

2019-03-22 14:39:12

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] bcache: Zero initialize bucket in bch_allocator_thread

On Fri, Mar 8, 2019 at 3:11 AM Coly Li <[email protected]> wrote:
> On 2019/3/8 6:10 上午, Nathan Chancellor wrote:
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/md/bcache/alloc.c:333:4: warning: variable 'bucket' is used
> > uninitialized whenever 'if' condition is false
> > [-Wsometimes-uninitialized]
> >
> > fifo_pop expands to a conditional depending on '!fifo_empty(...)', which
> > is the same condition as the while loop. It seems Clang can't piece
> > together that this condition will never been false as long as we are in
> > the while loop because they are the same condition. It's easy enough to
> > silence this warning by just explicitly zero initializing bucket.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/396
> > Signed-off-by: Nathan Chancellor <[email protected]>
>
> Hi Nathan,
>
> Nice catch, I will add it to my for-test. Thanks.

I saw the same warning but came up with a nicer (IMHO) fix, see
https://lore.kernel.org/lkml/[email protected]/T/#u

Arnd