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

[wdoekes]

> On Sept. 19, 2011, 8:16 a.m., wdoekes wrote:
> > trunk/res/res_agi.c, lines 3196-3199
> > <https://reviewboard.asterisk.org/r/1441/diff/2/?file=20685#file20685line3196>
> >
> >     Er. There is no reason for these casts. They were originally here because of the const nature of e->summary and friends.
> >     
> >     (The compiler would warn "warning: passing argument 1 of ‘free’ discards qualifiers from pointer target type".)
> >     
> >     But, ast_free already casts the arguments to void*, so the warning is gone anyway.
> >     
> >     Replace with ast_free(e->summary) and remove the "*((char **) &e->summary) = NULL;" below.
> >     
> >     P.S. schmidts: you'll also find these casts in func_odbc and possibly other modules that you might have missed because of missing deps.

P.S.2. You'd need to replace a = NULL with something like:

*(void**)&a = NULL;


- wdoekes


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


On Sept. 19, 2011, 3:23 a.m., schmidts wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> 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
> -----
> 
>   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/fd9cb5ca/attachment.htm>