[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