LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Chris Wright <chrisw@osdl.org>
To: Matthew Wilcox <willy@debian.org>
Cc: Chris Wright <chrisw@osdl.org>, Mingming Cao <cmm@us.ibm.com>,
	Andrew Morton <akpm@osdl.org>,
	ext2-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [Ext2-devel] Re: [PATCH] use-before-uninitialized value in ext3(2)_find_ goal
Date: Wed, 19 May 2004 15:32:49 -0700	[thread overview]
Message-ID: <20040519153249.N21045@build.pdx.osdl.net> (raw)
In-Reply-To: <20040519210603.GW6484@parcelfarce.linux.theplanet.co.uk>; from willy@debian.org on Wed, May 19, 2004 at 10:06:03PM +0100

* Matthew Wilcox (willy@debian.org) wrote:
> On Wed, May 19, 2004 at 12:53:28PM -0700, Chris Wright wrote:
> > I know it's a slightly bigger patch, but would it make sense to just enforce
> > this as part of api?  Just a thought...(patch untested)
> 
> No, that doesn't work.  Look:
> 
> reread:
> 	...
> 
>         if (ext2_find_goal(inode, iblock, chain, partial, &goal) < 0)
>                 goto changed;
> 
> changed:
>         while (partial > chain) {
>                 brelse(partial->bh);
>                 partial--;
>         }
>         goto reread;
> 
> So it's spaghetti code that can modify goal.  Yuck.
> 
> 5 labels in one function?  3 backwards jumps?  Disgusting.

Heh, yeah.  I actually did look, and had the same concern about goal.
I think it's ok though.  For one thing, in that changed->reread loop
goal is never used.  Secondly, I think that the intention was to have
*_find_goal start from 0, not from last goal, since goal is marked as
output, and Mingming's patch reset goal to 0 every pass through.  This
is exactly why I thought it useful to clarify the api.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

      reply	other threads:[~2004-05-21 23:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20040519043235.30d47edb.akpm@osdl.org>
2004-05-19 18:51 ` Mingming Cao
2004-05-19 19:53   ` Chris Wright
2004-05-19 20:33     ` [Ext2-devel] " Mingming Cao
2004-05-19 21:06     ` Matthew Wilcox
2004-05-19 22:32       ` Chris Wright [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=20040519153249.N21045@build.pdx.osdl.net \
    --to=chrisw@osdl.org \
    --cc=akpm@osdl.org \
    --cc=cmm@us.ibm.com \
    --cc=ext2-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=willy@debian.org \
    --subject='Re: [Ext2-devel] Re: [PATCH] use-before-uninitialized value in ext3(2)_find_ goal' \
    /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

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).