[asterisk-dev] [Code Review]: automagically set a pointer to null after it is freed with ast_free

[Kevin P. Fleming]

On 09/16/2011 10:14 AM, David Vossel wrote:
> ----- Original Message -----
>> From: "Simon Perreault"<simon.perreault at viagenie.ca>
>> To: "David Vossel"<dvossel at digium.com>
>> Cc: "Asterisk Developers"<asterisk-dev at lists.digium.com>, sst at sil.at
>> Sent: Friday, September 16, 2011 9:55:51 AM
>> Subject: Re: [Code Review]: automagically set a pointer to null after it is freed with ast_free
>> David Vossel wrote, on 09/16/2011 10:47 AM:
>>> I don't think we can change the behavior of ast_free(), but this
>>> functionality seems useful. I write the following lines all the
>>> time.
>>>
>>> ast_free(blah);
>>> blah = NULL;
>>
>> Well, you should stop doing that, for the reasons previously stated.
>>
>> Here's a "prisoner's dilemma-style" matrix to illustrate:
>>
>> | Set to null | Do not set to null
>> --------------------+-----------------+------------------------------------
>> Double free | Undetectable | Detectable with Valgrind or similar
>
> undetectable, because it won't happen.
>
>> Use of freed memory | Immediate crash | Detectable with Valgrind or
>> similar
>> As you can see, the "do not set to null" option is better overall.
>
> I'd rather have an immediate crash due to someone accessing a NULL pointer than someone accessing a pointer to freed memory.  In the pointer to freed memory case I have to use Valgrind to find the source of the problem which I don't have to do with the NULL pointer access.  Accessing a NULL pointer also produces immediate reproducible consistent crash results every time.
>
> This really comes down to one's own personal opinions and coding style.   I always clear pointers that point to freed memory.  I just can't see anything good coming from leaving dangling pointers around.

We have had similar discussions about pointers to astobj2 reference 
counted objects; it has been my opinion since we introduced astobj2 that 
releasing a reference to an object should clear the pointer that held 
the reference (although the code still does not actually do this). 
Having a dangling pointer to an ao2 object is extremely hard to debug, 
because the object may very well still be alive and usable... or it 
could be gone. It's much worse than the 'freed memory' situation, 
because at least in that situation if the memory is reused it will not 
be likely to appear to look like the same sort of data structure (at 
least in Asterisk... in systems that use pools/caches, reallocating the 
same type of data structure could end up using the same address as a 
previous instance).

-- 
Kevin P. Fleming
Digium, Inc. | Director of Software Technologies
Jabber: kfleming at digium.com | SIP: kpfleming at digium.com | Skype: kpfleming
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at www.digium.com & www.asterisk.org