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

[schmidts]

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

(Updated Sept. 19, 2011, 3:23 a.m.)


Review request for Asterisk Developers and David Vossel.


Changes
-------

i have changed the parts rmudgett said. thanks for this it works fine.

then i have tried to add a warning to astmm.c if a null pointer is freed and i see a lot of this warning coming for example from manager at startup and also channel.c on an incoming sip call. all this things looks like freeing an allready destroyed structure but i didnt get to deep into it to say if this happens cause of lazy style or if its really necessary. 

the discussion about this whole theme has some good opinions on both sides, it could lead to lazy programming style but on the other hand it make asterisk more stable.

for me it makes more sense throwing out a warning or an error message instead of crashing.


Summary
-------

first of all, this is just an idea so be friendly ;)

i have thought about setting a pointer to NULL after it is freed to
avoid segfaults and double free corruptions.

the most important change is to replace the define of ast_free to a
macro which just looks like this:

#define ast_free(x) ({ free((void *)x); x = NULL; })

and also to keep it working proberly change the define of ast_free_ptr
to free instead of ast_free.

the same change happens in astmm.h for malloc debugging calls:

#define ast_free(a) ({ __ast_free((void *) a,__FILE__, __LINE__, __PRETTY_FUNCTION__); a = NULL; })

after doing this changes i have seen a lot of places where a pointer is
cast at the ast_free call to remove the const state of it.
you can see in the diff all this places where this happens but i think
just cast it into void within the macro makes more sense.
because of this small changes the diff is so big, but its the same change on every part of it, except utils.h and astmm.h

only on three places i dont know how to handle this things:
in res_agi.c i had to replace ast_free with free to make it compile, i am sure there would be another way to solve this but i need some help with this.
in config.c i have the same problem like in res_agi.c

format_cap.c is allready cleared with dvossel


Diffs (updated)
-----

  trunk/main/frame.c 336161 
  trunk/main/features.c 336161 
  trunk/main/datastore.c 336161 
  trunk/main/event.c 336161 
  trunk/main/config.c 336161 
  trunk/apps/app_dial.c 336161 
  trunk/apps/app_voicemail.c 336161 
  trunk/channels/chan_sip.c 336161 
  trunk/include/asterisk/astmm.h 336161 
  trunk/include/asterisk/utils.h 336161 
  trunk/main/astmm.c 336161 
  trunk/main/ccss.c 336161 
  trunk/main/indications.c 336161 
  trunk/main/taskprocessor.c 336161 
  trunk/res/res_agi.c 336161 

Diff: https://reviewboard.asterisk.org/r/1441/diff


Testing
-------

it compiles with and without MALLOC_DEBUG,
it starts
and it do what it should do, the pointer is set to NULL after ast_free


Thanks,

schmidts

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110919/981dc4ab/attachment-0001.htm>