[Home]

Summary:ASTERISK-04563: [patch] fixes to vm_intro_it()
Reporter:Luigi Rizzo (rizzo)Labels:
Date Opened:2005-07-12 01:02:03Date Closed:2008-01-15 15:41:04.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_voicemail
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) vm.diff
Description:vm_intro_it() had both C-style bugs (whitespace instead of tabs
for indentation) and italian grammar bugs
(we normally say "nuovi messaggi" not "messaggi nuovi").
This patch:
- fixes the grammar;
- fixes the indentation (the whole body of the function has changed, so there
 was no reason to post a separate patch with whitespace change)
- on passing, uses a much more readable (my opinion, of course!) style to
 call multiple 'play' functions and deal with errors (see description in
 'Additional Information'.
The latter approach could be probably used in other parts of  this file.

****** ADDITIONAL INFORMATION ******

Instead of using sequences
 res = ast_play_and_wait(chan, "foo");
 if (!res)
     res = say_and_wait(chan, vms->newmessages, chan->language)
 if (!res)
     res = ast_play_and_wait(chan, "foo3");
 ...

I kept in mind that all these function return 0 on success, -1 on error, so
they can be concatenated with boolean ops.
Two catches: 1) && would be more intuitive on the success path, but
due to the negated logic (0 is success) you have to use || instead;
2) any non-zero value is converted to 1 in a boolean expression, so
we need to remember to convert it back at the end.
The result is something like this:
   res = (vms->oldmessages == 1) ?
            ast_play_and_wait(chan, "digits/un") ||
            ast_play_and_wait(chan, "vm-vecchio") ||
            ast_play_and_wait(chan, "vm-message") :
        /* 2 or more old messages */
            say_and_wait(chan, vms->oldmessages, chan->language) ||
            ast_play_and_wait(chan, "vm-vecchi") ||
            ast_play_and_wait(chan, "vm-messages");
with the message text not clutteded by the C structure.
Comments:By: Kevin P. Fleming (kpfleming) 2005-07-12 11:02:37

Committed to CVS HEAD with some minor formatting changes. Nice work!

By: Kevin P. Fleming (kpfleming) 2005-07-12 11:03:44

This patch included the uninitialized variable fix from ASTERISK-4644682, but I removed that before committing.

By: Digium Subversion (svnbot) 2008-01-15 15:41:04.000-0600

Repository: asterisk
Revision: 6105

U   trunk/apps/app_voicemail.c

------------------------------------------------------------------------
r6105 | kpfleming | 2008-01-15 15:41:04 -0600 (Tue, 15 Jan 2008) | 2 lines

clean up and reorganize vm_intro_it (bug ASTERISK-4563, with formatting changes)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=6105