[asterisk-dev] [Code Review] Switch public voicemail APIs over to use unique message IDs instead of message indexes.

[opticron]

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

Ship it!


Other than this, the changes look good.


/team/mmichelson/trunk-digiumphones/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1883/#comment11249>

    This return value should be checked.



/team/mmichelson/trunk-digiumphones/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1883/#comment11250>

    Ditto.



/team/mmichelson/trunk-digiumphones/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1883/#comment11251>

    Ditto.



/team/mmichelson/trunk-digiumphones/tests/test_voicemail_api.c
<https://reviewboard.asterisk.org/r/1883/#comment11252>

    Red blob.



/team/mmichelson/trunk-digiumphones/tests/test_voicemail_api.c
<https://reviewboard.asterisk.org/r/1883/#comment11253>

    old_msg_id is unused in this test and should be removed.


- opticron


On April 24, 2012, 11:46 a.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1883/
> -----------------------------------------------------------
> 
> (Updated April 24, 2012, 11:46 a.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Summary
> -------
> 
> Voicemail has some new public functions added to it in order to interoperate with Digium phones. These public functions give fine control over the ability to move, remove, forward, and play back messages. In the implementation in 1.8-digiumphones and 10-digiumphones, a message index is used to identify which message to operate on. This is potentially unreliable since mailboxes can be acted on by multiple sources. While the most likely result is that a user may try to manipulate a message that no longer exists, it is also possible that a user may end up manipulating an incorrect message instead. This could be disastrous depending on which wrong message is manipulated and how it is.
> 
> This patches the trunk version of app_voicemail to use unique message IDs instead of message indexes. It is not possible to manipulate the wrong message. The result of this is a performance decrease since there is more message retrieval from whatever storage is used, and there are more disk reads in order to check message IDs. A long-term goal would be to store message IDs in memory so that finding a message by its ID is not as expensive. However, this is a dark and treacherous road to go down and way beyond the scope of what needs to be done for now.
> 
> If there are additional changes that should be added with regards to the voicemail APIs, let me know.
> 
> 
> Diffs
> -----
> 
>   /team/mmichelson/trunk-digiumphones/apps/app_voicemail.c 363307 
>   /team/mmichelson/trunk-digiumphones/include/asterisk/app_voicemail.h 363307 
>   /team/mmichelson/trunk-digiumphones/tests/test_voicemail_api.c 363307 
> 
> Diff: https://reviewboard.asterisk.org/r/1883/diff
> 
> 
> Testing
> -------
> 
> As you'll see in this review, I have updated the unit tests in tests/test_voicemail_api.c. They all pass.
> 
> 
> Thanks,
> 
> Mark
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120503/12f37b8a/attachment.htm>