[vlc-devel] Re: [PATCH] - fix frees w/out NULLs

[Andrew Stone]

You make a good point about hiding double free()s. But are you sure valgrind won't detect this? Either way, I think you are right that it is not B&W. The biggest complaint seems to be that it adds unnecesarily to code where the pointer is not used again. I think this situation mostly shows up with local variables. So we could have a policy that you set the pointer to NULL *only* if it may be used again. 

-Andrew


----- Original Message ----
From: Rémi Denis-Courmont <rdenis at simphalempin.com>
To: vlc-devel at videolan.org
Sent: Friday, June 8, 2007 2:26:06 AM
Subject: [vlc-devel] Re: [PATCH] - fix frees w/out NULLs


On Thu, 7 Jun 2007 23:28:17 +0200, Pierre d'Herbemont <pdherbemont at free.fr>
wrote:
> But rather than implementing
> that way you could use a simple macro that redefine free() to set to
> NULL the pointer.

Such a macro would either:
- not be expansion-safe which is a big NO NO in that case,
- be a static inline that requires a pointer to the pointer which is
probably more overhead that we want.

I'd rather stick to manually setting NULL "when appropriate".

> My only concern is that we sometimes do really need
> to set the pointer to NULL whereas sometimes not.
> Such a patch will hide this subtlety for sure, which is not good.

Same here. There are cases where setting NULL is the right thing
(because further code expects it), some where it helps find bugs
(by crashing VLC instead of doing undefined stuff), some where it is
outright useless because the pointer is not ever used again, and some
where it hides bug (e.g. free(NULL) does nothing so you cannot detect
double-free bugs anymore, with say, valgrind).

It's not B&W...

-- 
Rémi Denis-Courmont
http://www.remlab.net/

-- 
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html





-- 
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html