[asterisk-dev] [Code Review] Bug fixes for voicemail user emailsubject / emailbody

[rmudgett]

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



/branches/10/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1374/#comment8071>

    Simply setting these pointers to NULL will cause a memory leak when this is not a new structure.  There are callers that have found or just created the structure.  If they just found it then this is a memory leak.



/branches/10/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1374/#comment8072>

    Possible memory leak if you do not free first.  The apply_option() is called by apply_options().
    
    You could perversely have:
    emailsubject=a subject
    emailbody=a body
    options=emailbody=new body|emailsubject=new subject



/branches/10/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1374/#comment8073>

    The memory leak may happen here as with apply_option().



/branches/10/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1374/#comment8070>

    The if NULL tests are not needed since ast_free will accept a NULL pointer.
    
    There are several other places in this file doing this as well.
    
    This is a cleanup that can be done in another patch.



/branches/10/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1374/#comment8067>

    There should be a case for '\\'.  Otherwise substitute_escape() will not allow the user to use \ in the string contents.
    
    This is not part of the issues being fixed, but it should be fixed.



/branches/10/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1374/#comment8068>

    Shouldn't emailsubject and pagersubject also be graced with substitue_escapes() here?


- rmudgett


On Aug. 18, 2011, 3:31 p.m., mjordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1374/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2011, 3:31 p.m.)
> 
> 
> Review request for Asterisk Developers and kmoore.
> 
> 
> Summary
> -------
> 
> This code change fixes two bugs:
> ASTERISK-16795
> ASTERISK-16781
> 
> ASTERISK-16795 was previously fixed in the last iteration and patched to 1.8 and trunk.  Unfortunately, a subsequent check-in blew out the change, removing it from 10 and trunk (10 being subsequently branched off of what was, at the time, trunk).  The previous fix for ASTERISK-16795 resolved most of ASTERISK-16781 - this code change fixes the rest of ASTERISK-16781 by applying escape options to all places voicemail user options are parsed out.
> 
> In order to complete this, the relevant code change in apply_options_full will be ported back to 1.8; the rest of these code changes are already in that branch.
> 
> 
> This addresses bugs ASTERISK-16781 and ASTERISK-16795;.
>     https://issues.asterisk.org/jira/browse/ASTERISK-16781
>     https://issues.asterisk.org/jira/browse/ASTERISK-16795;
> 
> 
> Diffs
> -----
> 
>   /branches/10/apps/app_voicemail.c 332504 
> 
> Diff: https://reviewboard.asterisk.org/r/1374/diff
> 
> 
> Testing
> -------
> 
> vmuser unit test updated and ran successfully.
> 
> 
> Thanks,
> 
> mjordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110824/7b9dcc1b/attachment-0001.htm>