Re: [PATCH] XFS: remove pointless conditional testing 'nmp' vs NULL in fs/xfs/xfs_rtalloc.c::xfs_growfs_rt()

["Jesper Juhl" <jesper.juhl@gmail.com>]

On 14/08/06, Nathan Scott <nathans@sgi.com> wrote:
> On Sun, Aug 13, 2006 at 12:16:50AM +0200, Jesper Juhl wrote:
> > In fs/xfs/xfs_rtalloc.c::xfs_growfs_rt() there's a completely useless
> > conditional at the error_exit label.
> > The 'if (nmp)' check is pointless and might as well be removed for two
> > reasons.
> >
> > 1) if 'nmp' is NULL then kmem_free() will end up calling kfree() with a NULL
> >    argument - which in turn will just cause a return from kfree(). No harm
> >    done.
>
> Thats valid.
>
> > 2) At the beginning of the function there's an assignment; '*nmp = *mp;' so
>
> Thats not.  Theres no assignment at the start of the function;
> theres one inside the main body of the loop 20+ lines into it,
> and right after a mem alloc with flags requiring no failure.
> Later that local variable is freed then set to NULL inside the
> loop, before continuing the next iteration...
>
> Really this code would be better if reworked slightly to just
> allocate nmp once before entering the loop, and then free it
> once at the end... we wouldn't need a goto, just a few breaks
> in the loop and a conditional transaction cancel.
>
> > This patch gets rid of the pointless check.
>
> Hmm, seems like code churn that makes the code slightly less
> obvious, but thats just me... I'd prefer a tested patch that
> implements the above suggestion, to be honest. :)
>
Ok, I'll see what I can come up with.

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html