Re: 2.6.24-rc5-mm1 - wonky disk cache and CDROM behavior...

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 19 Dec 2007 01:22:21 +0000 David Howells <dhowells@redhat.com> wrote:

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

Nobody seems to look after hppfs.  I'll resend the fat and hostfs patches to
maintainers for a review, please.