Re: [PATCH] don't do pointless NULL checks and casts before kfree() in security/

[Jesper Juhl <juhl-lkml@dif.dk>]

On Sun, 20 Mar 2005, Ralph Corderoy wrote:

> 
> Hi Jesper,
> 
> > kfree() handles NULL pointers, so checking a pointer for NULL before 
> > calling kfree() on it is pointless.
> 
> Not necessarily.  It helps tell the reader that the pointer may be NULL
> at that point.  This has come up before.
> 
>     http://groups-beta.google.com/group/linux.kernel/browse_thread/thread/bd3d6e5a29e43c73/7b43819f874295e8?q=ralph@inputplus.co.uk+persuade+lkml#7b43819f874295e8
> 

I agree that

	if (foo->bar) {
		kfree(foo->bar);
		foo->bar = NULL;
	}

makes it easy to see that foo->bar might be NULL, but I think the 
advantages of simply

	kfree(foo->bar);
	foo->bar = NULL;

outweigh that.

Having to remember that kfree(NULL) is valid shouldn't be hard, people 
should be used to that from userspace code calling free(), and if there 
are places where it's important to remember that the pointer might be 
NULL, then a simple comment would do, wouldn't it?

	kfree(foo->bar);	/* kfree(NULL) is valid */

the short version also have the real bennefits of generating shorter and 
faster code as well as being shorter "on-screen".


-- 
Jesper Juhl