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

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

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


Patch Set 3: Code-Review-1

(9 comments)

https://gerrit.asterisk.org/#/c/2883/3//COMMIT_MSG
Commit Message:

Line 22: ASTERISK-26045
Add #close since this patch resolves the issue.


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

Line 5191
It took awhile to find when this email header was added.  It was added in 475afdaf392693c2cf841c47606c5a58e1c44886 when IMAP was first added to app_voicemail.


PS3, Line 3265: 	ast_debug(5, " Mailbox %s", mailbox);
              : 	if (status->flags & SA_MESSAGES)
              : 		ast_debug(5, ", %lu messages", status->messages);
              : 	if (status->flags & SA_RECENT)
              : 		ast_debug(5, ", %lu recent", status->recent);
              : 	if (status->flags & SA_UNSEEN)
              : 		ast_debug(5, ", %lu unseen", status->unseen);
              : 	if (status->flags & SA_UIDVALIDITY)
              : 		ast_debug(5, ", %lu UID validity", status->uidvalidity);
              : 	if (status->flags & SA_UIDNEXT)
              : 		ast_debug(5, ", %lu next UID", status->uidnext);
              : 	ast_debug(5, "\n");
There needs to be \n's on each ast_log and ast_debug message created.  The logging system adds headers for each call that mess with the built up line attempted here.

Also nothing calls this function.  I'm slightly surprised that there isn't a warning about no prototype for this function.


PS3, Line 3504:                 if ((vms = pthread_getspecific(ts_vmstate.key)) && vms->username && vms->context &&
              :                     !strcmp(vms->username,mailbox) && !strcmp(vms->context, local_context)) {
              :                         return vms;
              :                 }
guidelines: Indent using tabs


PS3, Line 5154:         if (msgnum <= -1) {
              :                 fprintf(p, "Subject: New greeting '%s' on %s." ENDL, greeting_attachment, date);
guidelines: Indent using tabs


PS3, Line 5241:         if (msgnum <= -1) {
              :                 fprintf(p, "This message is to let you know that your greeting '%s' was changed on %s." ENDL
              :                                 "Please do not delete this message, lest your greeting vanish with it." ENDL ENDL,
              :                                 greeting_attachment, date);
guidelines: Indent using tabs


PS3, Line 7095: 
              :         /* get the current mailbox so that we can point the mailstream back to it later */
              :         curr_mbox = get_folder_by_name(vms->curbox);
              : 
guidelines: Indent using tabs


PS3, Line 7107:         /* restore previous mbox stream */
              :         if (init_mailstream(vms, curr_mbox) || !vms->mailstream) {
              :                 ast_log(AST_LOG_ERROR, "IMAP mailstream is NULL or can't init_mailstream\n");
              :                 res = -1;
              :         } else {
guidelines: Indent using tabs


Line 8118: 	const char mailbox_context[256] = "";
This doesn't need the "" initializer as the first time it is used it is initialized by a snprintf.  For character array initialization, assigning "" sets the whole array to zero.


-- 
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: 3
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-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes