[Asterisk-code-review] app voicemail: Add Mailbox Aliases (asterisk[16])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/10800 )

Change subject: app_voicemail:  Add Mailbox Aliases
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/#/c/10800/1/CHANGES
File CHANGES:

https://gerrit.asterisk.org/#/c/10800/1/CHANGES@18
PS1, Line 18:    which will allow you to create aliases for physical mailboxes. 
> Red space, just ignore if other comment doesn't require action :)

Weird.  I could have sworn I fixed this before I committed.


https://gerrit.asterisk.org/#/c/10800/1/apps/app_voicemail.c
File apps/app_voicemail.c:

https://gerrit.asterisk.org/#/c/10800/1/apps/app_voicemail.c@1804
PS1, Line 1804: 		mapping = ao2_find(alias_mailbox_mappings, search_string, OBJ_SEARCH_KEY);
              : 		if (mapping) {
              : 			char *search_mailbox;
              : 			char *search_context;
              : 
              : 			separate_mailbox(ast_strdupa(mapping->mailbox), &search_mailbox, &search_context);
              : 			vmu = find_user(ivm, search_mailbox, search_context);
              : 		}
> Should there be an ao2_ref call here? Just curious since it would
 > follow the pattern of the other calls like this.

ew, yeah.


https://gerrit.asterisk.org/#/c/10800/1/apps/app_voicemail.c@8157
PS1, Line 8157: 		ast_publish_mwi_state_channel(mailbox, context, new + urgent, old, channel_id);
> Would this potentially post a state update twice (or more than
 > twice) for a mailbox, specifically ones that can be found but also
 > have aliases for them?

yes but in this case, "mailbox" and "context" are reset to each alias so if 1234 at default had 2 new and 1 old message, and aliases for 4321 at devices and 9876 at devices, then we'd publish...
1234 at default 2,1
4321 at devices 2,1
9876 at devices 2,1



-- 
To view, visit https://gerrit.asterisk.org/10800
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: comment
Gerrit-Change-Id: I395b9205c91523a334fe971be0d1de4522067b04
Gerrit-Change-Number: 10800
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Tue, 18 Dec 2018 19:51:50 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181218/963acb7b/attachment.html>