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

[Andrew Stone]

Sorry about  
the space/tab thing. As for the macro, there already is one called FREENULL, which I found out after doing it manually for a while. So somebody must have thought it was useful to set pointers to null after they are freed :)

Cheers,
Andrew
 

----- Original Message ---- 

From: Pierre d'Herbemont <pdherbemont at free.fr> 

To: vlc-devel at videolan.org 

Sent: Thursday, June 7, 2007 5:28:17 PM 

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

 

 

 

On 7 juin 07, at 23:11, Andrew Stone wrote: 

 

Hi All, 

 

Here is a patch that goes through and sets a lot of pointers to NULL after they are freed. Please remember to do this, otherwise some nasty bugs can arise. It's much better to crash on a null pointer. 

 

Note that I only took care of a few files here, there are still thousands more of these to be fixed. 

 

 

 

 

Index: src/misc/objects.c 

-    if( p_this->psz_header ) free( p_this->psz_header ); 

+    if( p_this->psz_header )  

+      { 

+	free( p_this->psz_header ); 

+	p_this->psz_header = NULL; 

+      } 

 

 

Seems that you are mixing space/tabs here (more occurence in the rest of the patch though). Spaces should be used. 

 

 

Also, I am not sure we really want to do this. For instance in the above this is absolutely unecessary as we won't reuse the ptr. So I guess your patch add a lot of junk code. 

 

However you're point is interesting. But rather than implementing that way you could use a simple macro that redefine free() to set to NULL the pointer. 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. 

 

 

Pierre. 

 

 

 

 

 

 

 

 





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