[asterisk-dev] [Code Review]: Added VM_INFO dialplan function to return voicemail user information

[shawkris]

> On Nov. 8, 2011, 2:32 a.m., wdoekes wrote:
> > trunk/apps/app_voicemail.c, lines 11131-11132
> > <https://reviewboard.asterisk.org/r/1568/diff/2/?file=21648#file21648line11131>
> >
> >     messagecount takes a const char*, so you don't need the ast_strdupa.
> >     
> >     Furthermore, it already does an ast_strlen_zero(folder), so you don't need the S_OR either.
> >     
> >     If anything, you should do S_OR(arg.mailbox_folder, NULL) as the ODBC_STORAGE messagecount() doesn't do an ast_strlen_zero, but a boolean test. But you should probably fix that instead.
> 
> shawkris wrote:
>     The problem I was trying to solve here was a segfault if the folder is NULL, instead of an empty string.
> 
> wdoekes wrote:
>     I don't see where it would segfault. I have this:
>     
>     static int messagecount(const char *context, const char *mailbox, const char *folder)
>     {
>             if (ast_strlen_zero(folder) || !strcmp(folder, "INBOX")) {
>                     return __messagecount(context, mailbox, "INBOX") + __messagecount(context, mailbox, "Urgent");
>             } else {
>                     return __messagecount(context, mailbox, folder);
>             }
>     }
>     
>     And this:
>     
>     
>     static int messagecount(const char *context, const char *mailbox, const char *folder)
>     {
>     ...
>     
>             if (!folder)
>                     folder = "INBOX";
>     
>     And this:
>     
>     
>     static int messagecount(const char *context, const char *mailbox, const char *folder)
>     {
>             return __has_voicemail(context, mailbox, folder, 0) + (folder && strcmp(folder, "INBOX") ? 0 : __has_voicemail(context, mailbox, "Urgent", 0));
>     }
>     
>     Where __has_voicemail does this:
>     
>             if (ast_strlen_zero(folder))
>                     folder = "INBOX";
>     
>     
>     Where is the segfault coming from?

Actually I see now it was my use of ast_strdupa - if you pass a NULL it will segfault. As you say, I don't need to use ast_strdupa as it's not being parsed.


- shawkris


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


On Nov. 7, 2011, 7:04 p.m., shawkris wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1568/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2011, 7:04 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Creted VM_INFO dialplan function which can return information about a user defined in the standard voicemail application.
> 
> Supported attributes are: email, exists, fullname, pager, password, count.
> 
> e.g. from the dialplan, ${VM_INFO(1234 at test,email)} would return the email address for voicemail user 1234 in the test context.
> 
> The 'count' attribute can also take a folder parameter and will return the number of messages in that folder, or by default the Inbox.
> 
> 
> This addresses bug ASTERISK-18634.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18634
> 
> 
> Diffs
> -----
> 
>   trunk/CHANGES 343636 
>   trunk/UPGRADE.txt 343636 
>   trunk/apps/app_voicemail.c 343636 
> 
> Diff: https://reviewboard.asterisk.org/r/1568/diff
> 
> 
> Testing
> -------
> 
> Tested with a local dialplan calling the VM_INFO function with each parameter to make sure the expected result was returned. Tested that calling with an invalid attribute parameter produced an error message.
> 
> 
> Thanks,
> 
> shawkris
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111108/4390bd57/attachment.htm>