linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Omar Sandoval <osandov@osandov.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Arnd Bergmann <arnd@arndb.de>, Omar Sandoval <osandov@fb.com>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sbitmap: avoid maybe-uninitialized warning
Date: Mon, 19 Sep 2016 08:41:23 -0700	[thread overview]
Message-ID: <20160919154123.GA5193@vader> (raw)
In-Reply-To: <940657ab-4092-cebd-127e-6628adcf96b5@kernel.dk>

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

      reply	other threads:[~2016-09-19 15:41 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19 12:33 [PATCH] sbitmap: avoid maybe-uninitialized warning Arnd Bergmann
2016-09-19 14:43 ` Jens Axboe
2016-09-19 15:14   ` Arnd Bergmann
2016-09-19 15:22     ` Jens Axboe
2016-09-19 15:41       ` Omar Sandoval [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160919154123.GA5193@vader \
    --to=osandov@osandov.com \
    --cc=arnd@arndb.de \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=osandov@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).