Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <20071217185213.d0d02149.akpm@linux-foundation.org>
References: <20071217185213.d0d02149.akpm@linux-foundation.org> <20071213024050.7d6e5f3e.akpm@linux-foundation.org> <30457.1197931451@turing-police.cc.vt.edu> <20071217145644.2d6374e4.akpm@linux-foundation.org> <11403.1197943676@turing-police.cc.vt.edu> <20071218023732.GA3215@darkstar.te-china.tietoenator.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: dhowells@redhat.com, Dave Young <hidave.darkstar@gmail.com>,
       Valdis.Kletnieks@vt.edu, viro@zeniv.linux.org.uk,
       linux-kernel@vger.kernel.org
Subject: Re: 2.6.24-rc5-mm1 - wonky disk cache and CDROM behavior...
Date: Wed, 19 Dec 2007 01:22:21 +0000
Message-ID: <26475.1198027341@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1426
Lines: 39

Andrew Morton <akpm@linux-foundation.org> wrote:

> > -			inode = ERR_PTR(ret);
> > +			return NULL;
> >  		} else {
> >  			unlock_new_inode(inode);
> >  		}
> > 
> 
> Yup.

Nope.  The correct fix is to make the various callers use IS_ERR() to check
the result of this function rather than checking for a NULL return.

> David, this is concerning.  More such error-path bugs in that code will take
> years and years to get found and fixed.

Yes, I know.  I've looked over the patches several times, however I know there
may be bugs in there because I may have made assumptions about what I've
written that cause me to overlook things.  It's a danger of checking your own
code:-(

> The best way to eliminate them is a line-by-line re-review of the patchset.

And ideally by someone other than me.  Some of them have been reviewed by
other people, but I'm not sure that all have.

However, I've just had another look through.  ISOFS appears to be the only one
in which I'd missed updating the callers.  I've sent you a patch for it.

Note that I expressed reservations about three filesystems in the cover note
(FAT, HPPFS and HOSTFS), but nothing seems to have come of it.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/