[asterisk-dev] [Code Review] Prevent duplicate entries in format strings used in app_voicemail

[Russell Bryant]

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



/branches/1.4/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/429/#comment2927>

    I have seen some bugs before caused by using ast_strdupa() directly in an argument list.  Since this results in an alloca, it results in some bizarre bad behavior.
    
    Just do it before calling the function, store the result, and pass that.



/branches/1.4/main/file.c
<https://reviewboard.asterisk.org/r/429/#comment2928>

    Since this isn't performance critical code anything, we might as well just use snprintf(), or we'll hear about it from people running static code analysis and such that blindly yell when you use functions like this.


- Russell


On 2009-11-16 14:43:29, Matthew Nicholson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/429/
> -----------------------------------------------------------
> 
> (Updated 2009-11-16 14:43:29)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch adds the ast_format_str_reduce() function that will parse a '|' separated format string and remove any duplicate entries.  I am looking for feedback on the algorithm used here.  I believe it to be correct, but it is not very efficient and requires traversing the format list once for every format found in the format string.  This function is only called once (thus far), for each instance of the asterisk process, so efficiency is not the chief concern here, but any improvements are welcome.
> 
> This change is necessary because specifying the same format twice in app_voicemail will cause perpending a forwarded message to infinitely loop and fill up the hard disk as it prepends a message to itself.  Also, there is no reason that I can think of to specify the same format twice.
> 
> 
> This addresses bug 15625.
>     https://issues.asterisk.org/view.php?id=15625
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/apps/app_voicemail.c 229747 
>   /branches/1.4/include/asterisk/file.h 229747 
>   /branches/1.4/main/app.c 229747 
>   /branches/1.4/main/file.c 229747 
> 
> Diff: https://reviewboard.asterisk.org/r/429/diff
> 
> 
> Testing
> -------
> 
> Tested with the string "WAV|wav49|gsm".  WAV and wav49 are the same formats, the string is reduced to "WAV|gsm".
> 
> 
> Thanks,
> 
> Matthew
> 
>