[Asterisk-code-review] app voicemail: fix bugs, imap mm status log change to debug (asterisk[13])

Corey Farrell asteriskteam at digium.com
Tue May 24 11:48:13 CDT 2016


Corey Farrell has posted comments on this change.

Change subject: app_voicemail: fix bugs, imap mm_status log change to debug
......................................................................


Patch Set 1: Code-Review-1

(10 comments)

I have not done a technical review of the changes, only for coding guidelines.  Please take a look at the Asterisk Wiki for details about the expected formatting.

https://wiki.asterisk.org/wiki/display/AST/Coding+Guidelines

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

PS1, Line 2715: 		chan?S_COR(ast_channel_caller(chan)->id.number.valid, ast_channel_caller(chan)->id.number.str, NULL):NULL,
              : 		chan?S_COR(ast_channel_caller(chan)->id.name.valid, ast_channel_caller(chan)->id.name.str, NULL):NULL,
Need spaces around the '?' and ':' on both lines.


Line 3025: 	if (vms->mailstream && !mail_status (vms->mailstream, tmp, SA_UIDNEXT)) {
Guidelines: No space between function name and opening parenthesis.


Line 3122: 	if (!filename || !buffer) return;
Coding guidelines: brackets {} and line feeds are always required for conditionals, even when it's not required by C.  I know this is poorly followed in some old code but we ensure all new code and changes follow the rules.


Line 3124: 	if (!(output = fopen (filename, "w"))) {
No space after fopen.


Line 7099: 	if (vms->mailstream && !mail_status (vms->mailstream, mailbox, SA_UIDNEXT)) {
Space after function call.


Line 8238: #ifdef IMAP_STORAGE
Appears this entire block uses spaces to indent, coding guidelines require that tabs be used.


Line 8879:         curr_mbox = get_folder_by_name(vms->curbox);
Indent with tabs.


Line 8905:         if (curr_mbox != -1) {
Indent with tabs.


Line 11947:                         /* Reopen play_folder */
Indent with tabs.


Line 11949:                         if (res < 0)
Please add brackets {} around the conditional code.


-- 
To view, visit https://gerrit.asterisk.org/2883
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7f83d88b69b36934e2539c114b9fb612deed971b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list