public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ftp.linux.org.uk>
To: Zan Lynx <zlynx@acm.org>
Cc: Jens Axboe <axboe@suse.de>, Jesper Juhl <jesper.juhl@gmail.com>,
	Khushil Dep <khushil.dep@help.basilica.co.uk>,
	Luiz Fernando Capitulino <lcapitulino@mandriva.com.br>,
	akpm <akpm@osdl.org>, lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] bio: gcc warning fix.
Date: Fri, 6 Jan 2006 19:56:31 +0000	[thread overview]
Message-ID: <20060106195631.GW27946@ftp.linux.org.uk> (raw)
In-Reply-To: <1136576037.10342.6.camel@localhost>

On Fri, Jan 06, 2006 at 12:33:56PM -0700, Zan Lynx wrote:
> On Fri, 2006-01-06 at 19:48 +0100, Jens Axboe wrote:
> > On Fri, Jan 06 2006, Jesper Juhl wrote:
> > > gcc is right to warn in the sense that it doesn't know if
> > > bvec_alloc_bs() will read or write into idx when its address is passed
> > 
> > The function is right there, on top of bio_alloc_bioset(). It's even
> > inlined. gcc has absolutely no reason to complain.
> 
> GCC complains because it is possible for that function to return without
> ever setting a value into idx.  It's the "default" case in the switch.
> Of course, if that happens, idx will not be used and so it is not
> actually a problem.

If gcc would look at that code *after* it expands the call, it would
actually notice that everything's fine.  The codepath leaving the
inlined block without setting idx would look like

	bvl = NULL;
	goto l1;
...
l1:
	if (!bvl)
		goto l2;
	use idx
...
l2:
	mempool_free(bio, bs->bio_pool);
	bio = NULL;
	goto out;

and after that exit collapses into jump directly to l2, we end up with
situation when every path to use of idx obviously going through l1 and,
before that, end of switch() in inlined block.  All possible precursors
of that end of switch assign idx.

And yes, if you inline it manually gcc _will_ see that everything's OK.
Path that confuses it is
	default in switch -> exit from bio_alloc_bs() -> l1 -> use of idx
and 
		return value will be NULL	=>	  we will go to l2
is what it doesn't notice when it inlines itself.

  reply	other threads:[~2006-01-06 19:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-01-06 15:48 [PATCH] bio: gcc warning fix Khushil Dep
2006-01-06 18:41 ` Jesper Juhl
2006-01-06 18:48   ` Jens Axboe
2006-01-06 18:53     ` Jesper Juhl
2006-01-06 18:58     ` Luiz Fernando Capitulino
2006-01-06 19:04       ` Jens Axboe
2006-01-06 19:05       ` Jesper Juhl
2006-01-06 19:33     ` Zan Lynx
2006-01-06 19:56       ` Al Viro [this message]
2006-01-07 12:22         ` Peter Osterlund
  -- strict thread matches above, loose matches on Subject: below --
2006-01-06 15:07 Luiz Fernando Capitulino
2006-01-06 15:28 ` Jens Axboe
2006-01-06 15:39 ` Al Viro
2006-01-06 22:43   ` Daniel Barkalow
2006-01-06 22:52     ` Jesper Juhl
2006-01-06 23:13       ` Daniel Barkalow

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=20060106195631.GW27946@ftp.linux.org.uk \
    --to=viro@ftp.linux.org.uk \
    --cc=akpm@osdl.org \
    --cc=axboe@suse.de \
    --cc=jesper.juhl@gmail.com \
    --cc=khushil.dep@help.basilica.co.uk \
    --cc=lcapitulino@mandriva.com.br \
    --cc=linux-kernel@vger.kernel.org \
    --cc=zlynx@acm.org \
    /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