Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1486429 > unrolled thread

[PATCH] sbitmap: avoid maybe-uninitialized warning

Back to article view | Back to linux.kernel

Contents

  [PATCH] sbitmap: avoid maybe-uninitialized warning Arnd Bergmann <arnd@arndb.de> - 2016-09-19 14:40 +0200
    Re: [PATCH] sbitmap: avoid maybe-uninitialized warning Jens Axboe <axboe@kernel.dk> - 2016-09-19 16:50 +0200
      Re: [PATCH] sbitmap: avoid maybe-uninitialized warning Arnd Bergmann <arnd@arndb.de> - 2016-09-19 17:20 +0200
        Re: [PATCH] sbitmap: avoid maybe-uninitialized warning Jens Axboe <axboe@kernel.dk> - 2016-09-19 17:30 +0200
          Re: [PATCH] sbitmap: avoid maybe-uninitialized warning Omar Sandoval <osandov@osandov.com> - 2016-09-19 17:50 +0200

#1486429 — [PATCH] sbitmap: avoid maybe-uninitialized warning

The sbitmap code that has just been turned into a library module
returns uninitialized data for sbitmap_weight(), as pointed out by
gcc when building with -Wmaybe-uninitialized:

lib/sbitmap.c: In function 'sbitmap_weight':
lib/sbitmap.c:179:9: error: 'weight' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Note that the value is never initialized, we just add data on
top, so it is wrong regardless of sb->map_nr.

This adds the missing initialization.

Fixes: 88459642cba4 ("blk-mq: abstract tag allocation out into sbitmap library")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 lib/sbitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sbitmap.c b/lib/sbitmap.c
index e40808921544..2cecf05c82fd 100644
--- a/lib/sbitmap.c
+++ b/lib/sbitmap.c
@@ -169,7 +169,7 @@ EXPORT_SYMBOL_GPL(sbitmap_any_bit_clear);
 
 unsigned int sbitmap_weight(const struct sbitmap *sb)
 {
-	unsigned int i, weight;
+	unsigned int i, weight = 0;
 
 	for (i = 0; i < sb->map_nr; i++) {
 		const struct sbitmap_word *word = &sb->map[i];
-- 
2.9.0

[toc] | [next] | [standalone]

#1486551

On 09/19/2016 06:33 AM, Arnd Bergmann wrote:
> The sbitmap code that has just been turned into a library module
> returns uninitialized data for sbitmap_weight(), as pointed out by
> gcc when building with -Wmaybe-uninitialized:
>
> lib/sbitmap.c: In function 'sbitmap_weight':
> lib/sbitmap.c:179:9: error: 'weight' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> Note that the value is never initialized, we just add data on
> top, so it is wrong regardless of sb->map_nr.
>
> This adds the missing initialization.

Thanks Arnd, Colin sent the same patch and I applied it. Note that:

> Fixes: 88459642cba4 ("blk-mq: abstract tag allocation out into sbitmap library")

that isn't truly correct, that patch is just what moved the code. The 
bug predates that commit.

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]

#1486585

On Monday, September 19, 2016 8:43:12 AM CEST Jens Axboe wrote:
> On 09/19/2016 06:33 AM, Arnd Bergmann wrote:
> > The sbitmap code that has just been turned into a library module
> > returns uninitialized data for sbitmap_weight(), as pointed out by
> > gcc when building with -Wmaybe-uninitialized:
> >
> > lib/sbitmap.c: In function 'sbitmap_weight':
> > lib/sbitmap.c:179:9: error: 'weight' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > Note that the value is never initialized, we just add data on
> > top, so it is wrong regardless of sb->map_nr.
> >
> > This adds the missing initialization.
> 
> Thanks Arnd, Colin sent the same patch and I applied it. Note that:

Ok, thanks!

> > Fixes: 88459642cba4 ("blk-mq: abstract tag allocation out into sbitmap library")
> 
> that isn't truly correct, that patch is just what moved the code. The 
> bug predates that commit.

It's not important as long as the bug is fixed now, but I think it was
correct before that move, we just lost the 'used=0' initialization,
which also triggered the warning:

-static unsigned int bt_unused_tags(struct blk_mq_bitmap_tags *bt)
-{
-       unsigned int i, used;
-
-       for (i = 0, used = 0; i < bt->map_nr; i++) {
-               struct blk_align_bitmap *bm = &bt->map[i];
-
-               used += bitmap_weight(&bm->word, bm->depth);
        }
 
-       return bt->depth - used;
 }

	Arnd

[toc] | [prev] | [next] | [standalone]

#1486596

On 09/19/2016 09:14 AM, Arnd Bergmann wrote:
> On Monday, September 19, 2016 8:43:12 AM CEST Jens Axboe wrote:
>> On 09/19/2016 06:33 AM, Arnd Bergmann wrote:
>>> The sbitmap code that has just been turned into a library module
>>> returns uninitialized data for sbitmap_weight(), as pointed out by
>>> gcc when building with -Wmaybe-uninitialized:
>>>
>>> lib/sbitmap.c: In function 'sbitmap_weight':
>>> lib/sbitmap.c:179:9: error: 'weight' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>>
>>> Note that the value is never initialized, we just add data on
>>> top, so it is wrong regardless of sb->map_nr.
>>>
>>> This adds the missing initialization.
>>
>> Thanks Arnd, Colin sent the same patch and I applied it. Note that:
>
> Ok, thanks!
>
>>> Fixes: 88459642cba4 ("blk-mq: abstract tag allocation out into sbitmap library")
>>
>> that isn't truly correct, that patch is just what moved the code. The
>> bug predates that commit.
>
> It's not important as long as the bug is fixed now, but I think it was
> correct before that move, we just lost the 'used=0' initialization,
> which also triggered the warning:
>
> -static unsigned int bt_unused_tags(struct blk_mq_bitmap_tags *bt)
> -{
> -       unsigned int i, used;
> -
> -       for (i = 0, used = 0; i < bt->map_nr; i++) {
> -               struct blk_align_bitmap *bm = &bt->map[i];
> -
> -               used += bitmap_weight(&bm->word, bm->depth);
>         }
>
> -       return bt->depth - used;
>  }

I missed that, as I generally never use double inits in for loops, but I
guess I did for this one. But you are right, the bug was introduced with
the recent move, so your Fixes was completely correct.

Omar loses a cookie.

-- 
Jens Axboe

[toc] | [prev] | [next] | [standalone]

#1486610

On Mon, Sep 19, 2016 at 09:22:34AM -0600, Jens Axboe wrote:
> On 09/19/2016 09:14 AM, Arnd Bergmann wrote:
> > On Monday, September 19, 2016 8:43:12 AM CEST Jens Axboe wrote:
> > > On 09/19/2016 06:33 AM, Arnd Bergmann wrote:
> > > > The sbitmap code that has just been turned into a library module
> > > > returns uninitialized data for sbitmap_weight(), as pointed out by
> > > > gcc when building with -Wmaybe-uninitialized:
> > > > 
> > > > lib/sbitmap.c: In function 'sbitmap_weight':
> > > > lib/sbitmap.c:179:9: error: 'weight' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > > 
> > > > Note that the value is never initialized, we just add data on
> > > > top, so it is wrong regardless of sb->map_nr.
> > > > 
> > > > This adds the missing initialization.
> > > 
> > > Thanks Arnd, Colin sent the same patch and I applied it. Note that:
> > 
> > Ok, thanks!
> > 
> > > > Fixes: 88459642cba4 ("blk-mq: abstract tag allocation out into sbitmap library")
> > > 
> > > that isn't truly correct, that patch is just what moved the code. The
> > > bug predates that commit.
> > 
> > It's not important as long as the bug is fixed now, but I think it was
> > correct before that move, we just lost the 'used=0' initialization,
> > which also triggered the warning:
> > 
> > -static unsigned int bt_unused_tags(struct blk_mq_bitmap_tags *bt)
> > -{
> > -       unsigned int i, used;
> > -
> > -       for (i = 0, used = 0; i < bt->map_nr; i++) {
> > -               struct blk_align_bitmap *bm = &bt->map[i];
> > -
> > -               used += bitmap_weight(&bm->word, bm->depth);
> >         }
> > 
> > -       return bt->depth - used;
> >  }
> 
> I missed that, as I generally never use double inits in for loops, but I
> guess I did for this one. But you are right, the bug was introduced with
> the recent move, so your Fixes was completely correct.
> 
> Omar loses a cookie.

:(

Apparently we disable -Wmaybe-uninitialized by default since
6e8d666e9253 ("Disable "maybe-uninitialized" warning globally"), that's
probably why I didn't see this. My mistake, thanks Arnd and Colin.

-- 
Omar

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web