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

[Russell Bryant]

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

Ship it!


I think this is ready to go, with a couple of additional things:

1) I know you had a very nice test module for this code.  I don't see it in the diff, so be sure it makes its way into trunk.

2) Please document the required work to transition all dialplan functions over to the new code in doc/janitor-projects.txt


/trunk/apps/app_minivm.c
<http://reviewboard.digium.com/r/174/#comment1910>

    We should start marking internal docs with \internal at the beginning before the \brief.



/trunk/apps/app_minivm.c
<http://reviewboard.digium.com/r/174/#comment1909>

    Doxygen will treat "The" as a return value here.  In this case, just use \return.


- Russell


On 2009-04-02 10:54:38, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/174/
> -----------------------------------------------------------
> 
> (Updated 2009-04-02 10:54:38)
> 
> 
> 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 185911 
>   /trunk/apps/app_macro.c 185911 
>   /trunk/apps/app_minivm.c 185911 
>   /trunk/apps/app_voicemail.c 185911 
>   /trunk/cdr/cdr_custom.c 185911 
>   /trunk/funcs/func_aes.c 185911 
>   /trunk/funcs/func_base64.c 185911 
>   /trunk/funcs/func_blacklist.c 185911 
>   /trunk/funcs/func_callerid.c 185911 
>   /trunk/funcs/func_curl.c 185911 
>   /trunk/funcs/func_cut.c 185911 
>   /trunk/funcs/func_db.c 185911 
>   /trunk/funcs/func_dialplan.c 185911 
>   /trunk/funcs/func_env.c 185911 
>   /trunk/funcs/func_extstate.c 185911 
>   /trunk/funcs/func_groupcount.c 185911 
>   /trunk/funcs/func_lock.c 185911 
>   /trunk/funcs/func_logic.c 185911 
>   /trunk/funcs/func_md5.c 185911 
>   /trunk/funcs/func_module.c 185911 
>   /trunk/funcs/func_rand.c 185911 
>   /trunk/funcs/func_sha1.c 185911 
>   /trunk/funcs/func_speex.c 185911 
>   /trunk/funcs/func_strings.c 185911 
>   /trunk/funcs/func_sysinfo.c 185911 
>   /trunk/funcs/func_timeout.c 185911 
>   /trunk/funcs/func_vmcount.c 185911 
>   /trunk/include/asterisk/ast_expr.h 185911 
>   /trunk/include/asterisk/pbx.h 185912 
>   /trunk/main/ast_expr2f.c 185912 
>   /trunk/main/pbx.c 185911 
>   /trunk/main/strings.c 186021 
>   /trunk/res/res_agi.c 185911 
>   /trunk/res/res_config_curl.c 185911 
>   /trunk/res/res_phoneprov.c 185911 
> 
> Diff: http://reviewboard.digium.com/r/174/diff
> 
> 
> Testing
> -------
> 
> Extensive testing has been done on the new core API substitution call, and a module has been written to do testing of many places where functions have been written to take advantage of the new API.
> 
> 
> Thanks,
> 
> Tilghman
> 
>