[asterisk-dev] [Code Review] Create API for doing variable substitution with dynamic buffers

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/174/#review510
-----------------------------------------------------------


This isn't really a complete review, but I got distracted with a different issue while I was in the middle of looking at this. Here's what I have so far.


/trunk/apps/app_exec.c
<http://reviewboard.digium.com/r/174/#comment1171>

    There may be some potential danger to freeing args at this point. The reason is that when pbx_exec is called, chan's data field will be set to point to args. This reference to args may outlive the allocation lifetime of args.
    
    Of course, it wasn't really any better that before chan->data pointed to stacked data.



/trunk/apps/app_exec.c
<http://reviewboard.digium.com/r/174/#comment1172>

    This may sound strange after my previous note I left, but here, you don't free args at all, so you have a memory leak.



/trunk/main/pbx.c
<http://reviewboard.digium.com/r/174/#comment1175>

    I got a compilation warning when I tried to compile this. gcc complained that cp4 may be used uninitialized.


- Mark


On 2009-02-24 11:13:25, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/174/
> -----------------------------------------------------------
> 
> (Updated 2009-02-24 11:13:25)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> We currently have a variable substitution system that depends upon static buffers.  This has issues, both because it takes up 12k of stack each time through nested variable expressions, but also because in some newer cases, the 4k limit on the size of the buffer is no longer sufficient.  So this is a conversion that allows us to use dynamic string buffers in place of the old static buffers.
> 
> Additionally, note that this is a subset of the changes done, to make review of the changes easier.
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_exec.c 178266 
>   /trunk/include/asterisk/ast_expr.h 178266 
>   /trunk/include/asterisk/pbx.h 178266 
>   /trunk/main/ast_expr2f.c 178266 
>   /trunk/main/pbx.c 178266 
>   /trunk/main/strings.c 178266 
> 
> Diff: http://reviewboard.digium.com/r/174/diff
> 
> 
> Testing
> -------
> 
> Extensive testing has been done on the new core API substitution call, though limited testing has only been done on the multiple places in the code where the new API has replaced the old substitution.
> 
> 
> Thanks,
> 
> Tilghman
> 
>