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

[mjordan]

> On Aug. 25, 2011, 5:45 p.m., rmudgett wrote:
> > /branches/10/apps/app_voicemail.c, lines 1457-1458
> > <https://reviewboard.asterisk.org/r/1374/diff/3/?file=19198#file19198line1457>
> >
> >     Using calloc is a waste here as all bytes cleared by calloc are overwritten by the memcpy.
> 
> Kevin Fleming wrote:
>     You can also use structure assignment instead of memcpy() to copy structures; it's easier to read and will result in the same object code being generated by the compiler in nearly all cases.

Yeah, I was thinking about that as I was heading to my car.  I had originally broken this up more where one path didn't get the memcpy, then reduced it again.  I'll put it back to malloc in the morning.


> On Aug. 25, 2011, 5:45 p.m., rmudgett wrote:
> > /branches/10/apps/app_voicemail.c, lines 1459-1462
> > <https://reviewboard.asterisk.org/r/1374/diff/3/?file=19198#file19198line1459>
> >
> >     These aren't the only strings.  Isn't there a pagerbody and pagersubject equivalent?

You would think so, but the only two char * in the vmu struct are emailbody and emailsubject - pagerbody and pagersubject cannot be overriden on a user by user basis.  Chalk it up to one more inconsistency.


- mjordan


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


On Aug. 25, 2011, 4:55 p.m., mjordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1374/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2011, 4:55 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 333332 
> 
> 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/20110826/eb533e74/attachment.htm>