Summary:ASTERISK-06538: [patch][post 1.4] voicemail external notification not working
Reporter:Ryan Hulsker (rhulsker)Labels:
Date Opened:2006-03-13 17:03:41.000-0600Date Closed:2011-06-07 14:10:46
Versions:Frequency of
Environment:Attachments:( 0) voicemail_changes.diff
Description:Due to a typo in app_voicemail.c external notification scripts never get run.


Line 2350 in app_voicemail.c reads
if (messagecount(ext_context, &newvoicemails, &oldvoicemails)) {

When it should read
if (!messagecount(ext_context, &newvoicemails, &oldvoicemails)) {

Because messagecount() returns 0 on failure and 1 on success.
Comments:By: Olle Johansson (oej) 2006-03-14 01:07:54.000-0600

Messagecount returns 0 on success, -1 on some failures

By: Ryan Hulsker (rhulsker) 2006-03-14 11:47:56.000-0600

Well, that explains alot.

It looks like the version of messgagecount() that is used with ODBC_STORAGE is different from the messagecount() used without ODBC_STORAGE.

under ODBC_STORAGE messagecount returns 0 on failure, and 1 on success.

without ODBC_STORAGE messgaecount returns -1 on failure and 0 on success.

except in the case where no mailbox is passed in which case they both return 0.

The other difference is that the ODBC_STORAGE messagecount does not handle the case where multiple comma separated mailboxes are passed to the function.

I will work on a patch.

By: Ryan Hulsker (rhulsker) 2006-03-14 12:38:21.000-0600

Patch contains several simple fixes to messagecount() when used with ODBC_STORAGE.

ODBC_STORAGE version of messagecount() now has the same return behavior as the non ODBC_STORAGE version (ie 0=ok, -1=fail).

ODBC_STORAGE version of messagecount() now properly handles comma separated list of mailboxes to check (as in non ODBC_STORAGE)

ODBC_STORAGE version of messagecount() now properly handles NULL inputs for *newmsgs and *oldmsgs ie. don't check (as in non ODBC_STORAGE)

Sorry for the large amount of whitespace changes, this is due to wrapping significant portions of the code in if(){} blocks to handle the NULL parameters.

By: Olle Johansson (oej) 2006-03-14 14:52:21.000-0600

I can't test this patch, but it looks like you got it right. Thanks for taking time to fix this!

By: Jason Parker (jparker) 2006-03-16 20:26:02.000-0600

Please use tabs instead of spaces in your patch, per the coding guidelines.

Also...why are you calling messagecount recursively?  You could probably do it in a cleaner way - instead of calling 2 queries per call.  It appears that you're just returning a newmsg count and an oldmsg count.

By: Olle Johansson (oej) 2006-03-17 01:24:10.000-0600

north: Look at the other non-odbc messagecount function. We need to implement them the same way. If we change one to be non-recursive, we need to change both.

The recursive part is for supporting multiple mailboxes in one mailbox=1234,4556,5555 line.

By: Jason Parker (jparker) 2006-03-17 23:16:16.000-0600

I guess I know why it's being called recursively.  I'm just thinking there might be a better way to go about it.  2 queries (setups and teardowns) per mailbox seems like...a lot.

By: Olle Johansson (oej) 2006-03-18 03:22:29.000-0600

north: Right, I see what you mean. Agree. Please try to come up with a better way to do it! Thanks.

By: Olle Johansson (oej) 2006-04-04 08:57:27

Ok, seems like we need to stick with the current code since we don't get any clever new code. There's still time for that.

By: Olle Johansson (oej) 2006-04-04 08:58:25

We need an updated patch with formatting according to coding guidelines and applicable to current trunk version, thanks, rhulsker. I'm including this in test-this-branch as soon as I get that.

By: Serge Vecher (serge-v) 2006-05-04 10:00:44

please update the patch as per OEJ, so that this has a chance to make it into 1.4. Thanks!

By: Serge Vecher (serge-v) 2006-05-26 11:38:08

last try: rhulsker please update the patch very soon for it to make it into 1.4. thank you

By: Tilghman Lesher (tilghman) 2006-09-21 17:43:02

This appears to have been already fixed, in a different way.  Please ask a bug marshal to reopen this bug if it does not appear to be working for you.