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

[Tony Mountifield]

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.

It would be much better if the beginning of __ast_free_region(void *ptr,...)
was changed from:

if (!ptr) return;

to:

if (!ptr) crash();

This, combined with the OP's proposal to NULL freed pointers, would give
immediate notification of double frees, with crashes occurring much closer
to their root cause.

Cheers
Tony
-- 
Tony Mountifield
Work: tony at softins.co.uk - http://www.softins.co.uk
Play: tony at mountifield.org - http://tony.mountifield.org