[linux-cifs-client] Re: [PATCH] cifs: remove useless cargo-cultchecks

[Dave Kleikamp]

On Wed, 2007-03-07 at 17:33 -0500, Wagner, Chris (GE Infra, Non-GE, US)
wrote:
> I have no opinion on this but it did spark some memories.  I'm
> thinking of the famous "impossible; can't happen" result that somehow,
> sometimes, comes true. :)

If you truly think that something shouldn't happen, but that it might, a
BUG_ON() may be appropriate.  But only if not detecting the condition
would lead to some failure that is hard to debug.  Dereferencing a null
pointer leads a trap that is just as debuggable as a BUG_ON().

The only time code should test for a null pointer and return gracefully
is when there really is a valid case where the pointer can be null.
Otherwise, it will paper over whatever bug led to the null pointer.

> -----Original Message-----
> From: linux-cifs-client-bounces+chris.wagner=ae.ge.com at lists.samba.org on behalf of Steven French
> Sent: Wed 3/7/07 1:51 PM
> To: Christoph Hellwig
> Cc: linux-cifs-client at lists.samba.org; linux-kernel at vger.kernel.org
> Subject: [linux-cifs-client] Re: [PATCH] cifs: remove useless cargo-cultchecks
>  
> > file->f_path.dentry or file->f_path.dentry.d_inode can't be NULL
> 
> OK - I don't really mind removing these checks - and I agree that I there
> is not an obvious way that they can be null, yet we had a case in which
> file->f_dentry was reported as NULL a few years back in a bug report that
> we could not reproduce.

-- 
David Kleikamp
IBM Linux Technology Center