[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 4: Code-Review-1

(1 comment)

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

PS4, Line 3265: 	struct ast_str *str = ast_str_create(MAX_OBJECT_FIELD);
              : 
              : 	ast_str_append(&str, 0, " Mailbox %s", mailbox);
              : 	if (status->flags & SA_MESSAGES)
              : 		ast_str_append(&str, 0, ", %lu messages", status->messages);
              : 	if (status->flags & SA_RECENT)
              : 		ast_str_append(&str, 0, ", %lu recent", status->recent);
              : 	if (status->flags & SA_UNSEEN)
              : 		ast_str_append(&str, 0, ", %lu unseen", status->unseen);
              : 	if (status->flags & SA_UIDVALIDITY)
              : 		ast_str_append(&str, 0, ", %lu UID validity", status->uidvalidity);
              : 	if (status->flags & SA_UIDNEXT)
              : 		ast_str_append(&str, 0, ", %lu next UID", status->uidnext);
              : 	ast_debug(5, "%s\n", ast_str_buffer(str));
              : 
              : 	ast_free(str);
Since you are going this route.  ast_str_create() can fail because it mallocs memory.

Avoid always building str just to throw it away if debug is not at level 5 or above.

func()
{
  struct ast_str *str;

  if (!DEBUG_ATLEAST(5) || !(str = ast_str_create(MAX_OBJECT_FIELD))) {
    return;
  }

  ast_str_append(&str, ...);
  if (status->flags & SA_MESSAGE) {
    ast_str_append(&str, 0, ", %lu messages", status->messages);
  }
  ...
  ast_log(LOG_DEBUG, "%s\n", ast_str_buffer(str));
  ast_free(str);
}

You may as well add the curly braces required by the guidelines for the if statements since you are basically rewriting this function.


-- 
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: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Reviewer: 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