[asterisk-dev] [Code Review] MALLOC_HOLD for debugging memory

[Mark Michelson]

> On 2009-07-13 15:51:48, Mark Michelson wrote:
> > I question the usefulness of this change mainly because you can't see where in the code the illegal write to memory occurred. This also does not allow for the detection of illegal reads of freed memory.
> > 
> > Another potential problem (although this would never occur if Asterisk were perfect) is that the method used here is not thread-safe. You iterate over each byte of memory and see if the memory is non-NULL. The problem is that a section you already read could be written to after you pass over it. In order for this to work correctly, every write to memory would have to grab the reglock. That simply is not feasible.

Another problem is that this change masks any double frees which occur. A nice upside to this is that with a small bit of modification, you can actually detect and report double frees pretty easily with this code change.

(I would have mentioned this earlier, but review board was adamantly refusing to let me edit my review further)


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/309/#review966
-----------------------------------------------------------


On 2009-07-13 14:42:42, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/309/
> -----------------------------------------------------------
> 
> (Updated 2009-07-13 14:42:42)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> MALLOC_HOLD is an optional compile-time definition used to detect memory corruption.  It does this in a similar way to valgrind, although it does not slow down Asterisk appreciably (although the memory will grow somewhat larger than normal).
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/build_tools/cflags.xml 206144 
>   /branches/1.4/main/astmm.c 206144 
> 
> Diff: https://reviewboard.asterisk.org/r/309/diff
> 
> 
> Testing
> -------
> 
> Extensive testing with an idle system.
> 
> 
> Thanks,
> 
> Tilghman
> 
>