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

[David Howells]

Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/