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

[Kevin P. Fleming]

On 09/16/2011 04:15 AM, Tony Mountifield wrote:
> In article<20110916000323.17691.46179 at hotblack.digium.com>,
> Simon Perreault<reviewboard at asterisk.org>  wrote:
>>
>> I have a major problem with this. It hides bugs rather than fixing them. Double frees and
>> other such problems are coding errors that are not simply fixed by setting the pointer to
>> zero. If you double free a pointer, odds are there are deeper issues in your code that need
>> to be fixed. Crashing is a good indicator that there is something that needs to be fixed. If
>> you set the pointer to zero, you trade an often immediate crash for a later crash.
>
> It could be the other way round: continuing to dereference a pointer after
> its referent has been freed may well continue to work for a while, and if
> the referred-to area gets reused, behaviour can be undefined and appear
> quite unrelated to the root cause of the error. If the pointer is set to
> NULL when freed, any attempt to dereference it will immediately crash,
> making the problem much easier to diagnose.
>
> The big problem is that both free() and __ast_free_region() act as no-ops
> when passed a NULL pointer. Is there ANY situation where this is really
> desired behaviour? Any attempt to pass NULL to free() and similar is
> surely indicative of a bug.

That is standard behavior for free(), and a lot of code relies on it. 
Without it, every place that has a pointer that may or may not have a 
value in it has to check first to see if it is NULL before calling 
free()... and you end up with hundreds of tests for NULL instead of one 
in the free() function (or its equivalent).

-- 
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