[asterisk-dev] [Code Review] Fix memory leak in manager.c action originate when using channel variables

[Olle E Johansson]

> On 2010-08-26 22:14:14, Terry Wilson wrote:
> > It looks like ast_pbx_outgoing_app/exten call ast_variables_destroy() in all circumstances, so I don't think this patch is necessary (and it is also why it crashes--vars is never set to NULL but so that check fails and then ast_variables_destroy is called again). But, I did notice that ast_variables_destroy() calls free() instead of ast_free() even though ast_variable_new() calls allocates with ast_calloc().
> 
> Olle E Johansson wrote:
>     I see what you mean, but there are also a lot of cases where we do not call ast_pbx_outgoign_app and still have these vars allocated, so there are still areas to fix, but in a more limited scope. I'll go throught it and put up a new diff for review.
> 
> Terry Wilson wrote:
>     Yes. Just moving the variable assignment down past error returns and covering the situation where if(exten && context && pi) fails should do it. fast_originate calls one of the ast_pbx_outgoing_* functions in every case. Copying the variables shouldn't be necessary.

Exactly. THanks for the feedback.


- Olle E


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


On 2010-08-20 07:41:17, Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/869/
> -----------------------------------------------------------
> 
> (Updated 2010-08-20 07:41:17)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The originate action supports using channel variables. These are allocated to a variable "vars" but never freed. In the case of asynchronos or "fast" originate, these are handed off as a pointer to the new thread.
> 
> This patch 
>  - Moves the allocation of the variables until after a few checks that will return (to not have to free multiple times).
>  - Frees the variables in originate
>  - copies the variables to the new thread, which frees it's own copy when done
> 
> Separate fix is made for trunk. The function ast_variable_copy really belongs to config.c/h, but since we don't change API in releases, it's in manager.c for now, and will move home in the trunk version. 
> 
> (Have fixed the red dots in my changes)
> 
> ----
> Trunk version now in 
> https://origsvn.digium.com/svn/asterisk/team/oej/bufo-manager-varfix-trunk
> 
> 
> This addresses bug 17891.
>     https://issues.asterisk.org/view.php?id=17891
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/main/manager.c 282966 
> 
> Diff: https://reviewboard.asterisk.org/r/869/diff
> 
> 
> Testing
> -------
> 
> Tested. Crashed. Will update to a working solution.
> 
> 
> Thanks,
> 
> Olle E
> 
>