[asterisk-commits] jpeeler: branch 1.4 r293004 - /branches/1.4/apps/app_voicemail.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon Oct 25 17:55:31 CDT 2010


Author: jpeeler
Date: Mon Oct 25 17:55:28 2010
New Revision: 293004

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=293004
Log:
Fix inprocess_container in voicemail to correctly restrict max messages.

The comparison function logic was off, so the number of sessions for a given
mailbox were not being incremented properly. This problem caused the maximum
number of messages per folder to not be respected when simultaneously leaving
multiple voicemails just below the threshold. 

These problems should be fixed by the above, but just in case:
Fixed resequence_mailbox to rely on the actual number of detected number of
files in a directory rather than just assuming only 10 messages more than the
maximum had been left. Also if more messages than the maximum are deleted they
are actually removed now.


The second purpose of this commit should have been separated out probably, but
is related to the above. Again, if the number of messages in a given voicemail
folder exceeds the maximum set limit make sure to allocate enough space for the
deleted and heard index tracking array.

A few random fixes:
There was a forgotten decrement of the inprocess count in imap_store_file.

When using IMAP storage, do not look in the directory where file based storage
messages may still reside and influence the message count.

Ensure to use only the first format in sendmail.

ABE-2516

Modified:
    branches/1.4/apps/app_voicemail.c

Modified: branches/1.4/apps/app_voicemail.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.4/apps/app_voicemail.c?view=diff&rev=293004&r1=293003&r2=293004
==============================================================================
--- branches/1.4/apps/app_voicemail.c (original)
+++ branches/1.4/apps/app_voicemail.c Mon Oct 25 17:55:28 2010
@@ -380,6 +380,7 @@
 	char fn2[PATH_MAX];
 	int *deleted;
 	int *heard;
+	int dh_arraysize; /* used for deleted / heard allocation */
 	int curmsg;
 	int lastmsg;
 	int newmessages;
@@ -431,7 +432,7 @@
 static int inprocess_cmp_fn(void *obj, void *arg, int flags)
 {
 	struct inprocess *i = obj, *j = arg;
-	if (!strcmp(i->mailbox, j->mailbox)) {
+	if (strcmp(i->mailbox, j->mailbox)) {
 		return 0;
 	}
 	return !strcmp(i->context, j->context) ? CMP_MATCH : 0;
@@ -449,6 +450,9 @@
 		ao2_unlock(inprocess_container);
 		ao2_ref(i, -1);
 		return ret;
+	}
+	if (delta < 0) {
+		ast_log(LOG_WARNING, "BUG: ref count decrement on non-existing object???\n");
 	}
 	if (!(i = ao2_alloc(sizeof(*i) + strlen(context) + strlen(mailbox) + 2, NULL))) {
 		ao2_unlock(inprocess_container);
@@ -1065,6 +1069,33 @@
 		free(vmu);
 }
 
+static int vm_allocate_dh(struct vm_state *vms, struct ast_vm_user *vmu, int count_msg) {
+
+	int arraysize = (vmu->maxmsg > count_msg ? vmu->maxmsg : count_msg);
+	if (!vms->dh_arraysize) {
+		/* initial allocation */
+		if (!(vms->deleted = ast_calloc(arraysize, sizeof(int)))) {
+			return -1;
+		}
+		if (!(vms->heard = ast_calloc(arraysize, sizeof(int)))) {
+			return -1;
+		}
+		vms->dh_arraysize = arraysize;
+	} else if (vms->dh_arraysize < arraysize) {
+		if (!(vms->deleted = ast_realloc(vms->deleted, arraysize * sizeof(int)))) {
+			return -1;
+		}
+		if (!(vms->heard = ast_realloc(vms->heard, arraysize * sizeof(int)))) {
+			return -1;
+		}
+		memset(vms->deleted, 0, arraysize * sizeof(int));
+		memset(vms->heard, 0, arraysize * sizeof(int));
+		vms->dh_arraysize = arraysize;
+	}
+
+	return 0;
+}
+
 /* All IMAP-specific functions should go in this block. This
  * keeps them from being spread out all over the code */
 #ifdef IMAP_STORAGE
@@ -1341,8 +1372,6 @@
 
 static int imap_check_limits(struct ast_channel *chan, struct vm_state *vms, struct ast_vm_user *vmu, int msgnum)
 {
-	int res;
-
 	/* Check if mailbox is full */
 	check_quota(vms, imapfolder);
 	if (vms->quota_limit && vms->quota_usage >= vms->quota_limit) {
@@ -1351,22 +1380,15 @@
 		ast_play_and_wait(chan, "vm-mailboxfull");
 		return -1;
 	}
+
+	/* Check if we have exceeded maxmsg */
 	if (option_debug > 2)
-		ast_log(LOG_DEBUG, "Checking message number quota - mailbox has %d messages, maximum is set to %d\n",msgnum,vmu->maxmsg);
-	if (msgnum >= vmu->maxmsg - inprocess_count(vmu->mailbox, vmu->context, 0)) {
-		res = ast_streamfile(chan, "vm-mailboxfull", chan->language);
-		if (!res)
-			res = ast_waitstream(chan, "");
-		ast_log(LOG_WARNING, "No more messages possible\n");
-		pbx_builtin_setvar_helper(chan, "VMSTATUS", "FAILED");
-		return -1;
-	}
-
-	/* Check if we have exceeded maxmsg */
+		ast_log(LOG_DEBUG, "Checking message number quota: mailbox has %d messages, maximum is set to %d, current messages %d\n", msgnum, vmu->maxmsg, inprocess_count(vmu->mailbox, vmu->context, 0));
 	if (msgnum >= vmu->maxmsg - inprocess_count(vmu->mailbox, vmu->context, +1)) {
-		ast_log(LOG_WARNING, "Unable to leave message since we will exceed the maximum number of messages allowed (%u > %u)\n", msgnum, vmu->maxmsg);
+		ast_log(LOG_WARNING, "Unable to leave message since we will exceed the maximum number of messages allowed (%u >= %u)\n", msgnum, vmu->maxmsg);
 		ast_play_and_wait(chan, "vm-mailboxfull");
 		inprocess_count(vmu->mailbox, vmu->context, -1);
+		pbx_builtin_setvar_helper(chan, "VMSTATUS", "FAILED");
 		return -1;
 	}
 	return 0;
@@ -1457,6 +1479,7 @@
 	}
 	if (tempcopy)
 		*(vmu->email) = '\0';
+	inprocess_count(vmu->mailbox, vmu->context, -1);
 	return 0;
 
 }
@@ -1713,6 +1736,16 @@
 	mail_search_full (vms->mailstream, NULL, pgm, NIL);
 
 	vms->lastmsg = vms->vmArrayIndex - 1;
+	/* Since IMAP storage actually stores both old and new messages in the same IMAP folder,
+	 * ensure to allocate enough space to account for all of them. Warn if old messages
+	 * have not been checked first as that is required.
+	 */
+	if (box == 0 && !vms->dh_arraysize) {
+		ast_log(LOG_WARNING, "The code expects the old messages to be checked first, fix the code.\n");
+	}
+	if (vm_allocate_dh(vms, vmu, box == 0 ? vms->vmArrayIndex + vms->oldmessages : vms->lastmsg)) {
+		return -1;
+	}
 
 	mail_free_searchpgm(&pgm);
 	ast_mutex_unlock(&vms->lock);
@@ -3572,11 +3605,18 @@
 	FILE *p=NULL;
 	char tmp[80] = "/tmp/astmail-XXXXXX";
 	char tmp2[256];
+	char *stringp;
 
 	if (vmu && ast_strlen_zero(vmu->email)) {
 		ast_log(LOG_WARNING, "E-mail address missing for mailbox [%s].  E-mail will not be sent.\n", vmu->mailbox);
 		return(0);
 	}
+
+	/* Mail only the first format */
+	format = ast_strdupa(format);
+	stringp = format;
+	strsep(&stringp, "|");
+
 	if (!strcmp(format, "wav49"))
 		format = "WAV";
 	if (option_debug > 2)
@@ -4179,7 +4219,14 @@
 		ast_copy_string(prefile, tempfile, sizeof(prefile));
 	DISPOSE(tempfile, -1);
 	/* It's easier just to try to make it than to check for its existence */
+#ifndef IMAP_STORAGE
 	create_dirpath(dir, sizeof(dir), vmu->context, ext, "INBOX");
+#else
+	snprintf(dir, sizeof(dir), "%simap", VM_SPOOL_DIR);
+	if (mkdir(dir, VOICEMAIL_DIR_MODE) && errno != EEXIST) {
+		ast_log(LOG_WARNING, "mkdir '%s' failed: %s\n", dir, strerror(errno));
+	}
+#endif
 	create_dirpath(tmpdir, sizeof(tmpdir), vmu->context, ext, "tmp");
 
 	/* Check current or macro-calling context for special extensions */
@@ -4305,7 +4352,7 @@
 		msgnum = newmsgs + oldmsgs;
 		if (option_debug > 2)
 			ast_log(LOG_DEBUG, "Messagecount set to %d\n",msgnum);
-		snprintf(fn, sizeof(fn), "%s/imap/msg%s%04d", VM_SPOOL_DIR, vmu->mailbox, msgnum);
+		snprintf(fn, sizeof(fn), "%simap/msg%s%04d", VM_SPOOL_DIR, vmu->mailbox, msgnum);
 		/* set variable for compatability */
 		pbx_builtin_setvar_helper(chan, "VM_MESSAGEFILE", "IMAP_STORAGE");
 
@@ -4471,7 +4518,7 @@
 }
 
 #ifndef IMAP_STORAGE
-static int resequence_mailbox(struct ast_vm_user *vmu, char *dir)
+static int resequence_mailbox(struct ast_vm_user *vmu, char *dir, int count_msg)
 {
 	/* we know max messages, so stop process when number is hit */
 
@@ -4482,7 +4529,7 @@
 	if (vm_lock_path(dir))
 		return ERROR_LOCK_PATH;
 
-	for (x = 0, dest = 0; x < vmu->maxmsg + 10; x++) {
+	for (x = 0, dest = 0; x < count_msg + 1; x++) {
 		make_file(sfn, sizeof(sfn), dir, x);
 		if (EXISTS(dir, x, sfn, NULL)) {
 			
@@ -4496,7 +4543,7 @@
 	}
 	ast_unlock_path(dir);
 
-	return 0;
+	return dest;
 }
 #endif
 
@@ -5214,7 +5261,11 @@
 	int newmsgs = 0, oldmsgs = 0;
 	const char *category = pbx_builtin_getvar_helper(chan, "VM_CATEGORY");
 
+#ifndef IMAP_STORAGE
 	make_dir(todir, sizeof(todir), vmu->context, vmu->mailbox, "INBOX");
+#else
+	snprintf(todir, sizeof(todir), "%simap", VM_SPOOL_DIR);
+#endif
 	make_file(fn, sizeof(fn), todir, msgnum);
 	snprintf(ext_context, sizeof(ext_context), "%s@%s", vmu->mailbox, vmu->context);
 
@@ -5898,11 +5949,16 @@
 	/* Faster to make the directory than to check if it exists. */
 	create_dirpath(vms->curdir, sizeof(vms->curdir), vmu->context, vms->username, vms->curbox);
 
+	/* traverses directory using readdir (or select query for ODBC) */
 	count_msg = count_messages(vmu, vms->curdir);
 	if (count_msg < 0)
 		return count_msg;
 	else
 		vms->lastmsg = count_msg - 1;
+
+	if (vm_allocate_dh(vms, vmu, count_msg)) {
+		return -1;
+	}
 
 	/*
 	The following test is needed in case sequencing gets messed up.
@@ -5911,15 +5967,14 @@
 	detected.
 	*/
 
+	/* for local storage, checks directory for messages up to maxmsg limit */
 	last_msg = last_message_index(vmu, vms->curdir);
 	if (last_msg < 0)
 		return last_msg;
 	else if (vms->lastmsg != last_msg)
 	{
-		ast_log(LOG_NOTICE, "Resequencing Mailbox: %s\n", vms->curdir);
-		res = resequence_mailbox(vmu, vms->curdir);
-		if (res)
-			return res;
+		ast_log(LOG_NOTICE, "Resequencing Mailbox: %s, expected %d but found %d message(s) in box with max threshold of %d.\n", vms->curdir, last_msg + 1, vms->lastmsg + 1, vmu->maxmsg);
+		res = resequence_mailbox(vmu, vms->curdir, count_msg);
 	}
 
 	return 0;
@@ -5942,7 +5997,8 @@
 	if (vm_lock_path(vms->curdir))
 		return ERROR_LOCK_PATH;
 	 
-	for (x = 0; x < vmu->maxmsg; x++) { 
+	/* must check up to last detected message, just in case it is erroneously greater than maxmsg */
+	for (x = 0; x < vms->lastmsg + 1; x++) { 
 		if (!vms->deleted[x] && (strcasecmp(vms->curbox, "INBOX") || !vms->heard[x])) { 
 			/* Save this message.  It's not in INBOX or hasn't been heard */ 
 			make_file(vms->fn, sizeof(vms->fn), vms->curdir, x); 
@@ -5978,7 +6034,7 @@
 	if (vms->deleted) {
 		/* Since we now expunge after each delete, deleting in reverse order
 		 * ensures that no reordering occurs between each step. */
-		for (x = vmu->maxmsg - 1; x >= 0; x--) {
+		for (x = vms->dh_arraysize - 1; x >= 0; x--) {
 			if (vms->deleted[x]) {
 				if (option_debug > 2) {
 					ast_log(LOG_DEBUG, "IMAP delete of %d\n", x);
@@ -5991,9 +6047,9 @@
 
 done:
 	if (vms->deleted)
-		memset(vms->deleted, 0, vmu->maxmsg * sizeof(int)); 
+		memset(vms->deleted, 0, vms->dh_arraysize * sizeof(int)); 
 	if (vms->heard)
-		memset(vms->heard, 0, vmu->maxmsg * sizeof(int)); 
+		memset(vms->heard, 0, vms->dh_arraysize * sizeof(int)); 
 
 	return 0;
 }
@@ -7652,15 +7708,7 @@
 	vmstate_insert(&vms);
 	init_vm_state(&vms);
 #endif
-	if (!(vms.deleted = ast_calloc(vmu->maxmsg, sizeof(int)))) {
-		/* TODO: Handle memory allocation failure */
-		goto out;
-	}
-	if (!(vms.heard = ast_calloc(vmu->maxmsg, sizeof(int)))) {
-		/* TODO: Handle memory allocation failure */
-		goto out;
-	}
-	
+
 	/* Set language from config to override channel language */
 	if (!ast_strlen_zero(vmu->language))
 		ast_string_field_set(chan, language, vmu->language);
@@ -7669,14 +7717,14 @@
 	if (option_debug)
 		ast_log(LOG_DEBUG, "Before open_mailbox\n");
 	res = open_mailbox(&vms, vmu, 1);
-	if (res == ERROR_LOCK_PATH)
+	if (res < 0)
 		goto out;
 	vms.oldmessages = vms.lastmsg + 1;
 	if (option_debug > 2)
 		ast_log(LOG_DEBUG, "Number of old messages: %d\n",vms.oldmessages);
 	/* Start in INBOX */
 	res = open_mailbox(&vms, vmu, 0);
-	if (res == ERROR_LOCK_PATH)
+	if (res < 0)
 		goto out;
 	vms.newmessages = vms.lastmsg + 1;
 	if (option_debug > 2)
@@ -7685,7 +7733,7 @@
 	/* Select proper mailbox FIRST!! */
 	if (play_auto) {
 		res = open_mailbox(&vms, vmu, play_folder);
-		if (res == ERROR_LOCK_PATH)
+		if (res < 0)
 			goto out;
 
 		/* If there are no new messages, inform the user and hangup */
@@ -7699,7 +7747,7 @@
 			/* If we only have old messages start here */
 			res = open_mailbox(&vms, vmu, 1);
 			play_folder = 1;
-			if (res == ERROR_LOCK_PATH)
+			if (res < 0)
 				goto out;
 		}
 	}
@@ -7768,7 +7816,7 @@
 				if (res == ERROR_LOCK_PATH)
 					goto out;
 				res = open_mailbox(&vms, vmu, cmd);
-				if (res == ERROR_LOCK_PATH)
+				if (res < 0)
 					goto out;
 				play_folder = cmd;
 				cmd = 0;




More information about the asterisk-commits mailing list