[asterisk-commits] russell: branch 1.4 r91780 - /branches/1.4/apps/app_queue.c

[SVN commits to the Asterisk project]

Author: russell
Date: Fri Dec  7 10:25:25 2007
New Revision: 91780

URL: http://svn.digium.com/view/asterisk?view=rev&rev=91780
Log:
* Add channel locking around datastore operations that expect the channel
  to be locked.
* Document why we don't record Local channels in the dialed interfaces list.
* Handle memory allocation failure.
* Remove the dialed variable, as it wasn't actually needed.
* Tweak some formatting to conform to coding guidelines.

Modified:
    branches/1.4/apps/app_queue.c

Modified: branches/1.4/apps/app_queue.c
URL: http://svn.digium.com/view/asterisk/branches/1.4/apps/app_queue.c?view=diff&rev=91780&r1=91779&r2=91780
==============================================================================
--- branches/1.4/apps/app_queue.c (original)
+++ branches/1.4/apps/app_queue.c Fri Dec  7 10:25:25 2007
@@ -2502,7 +2502,11 @@
 	int forwardsallowed = 1;
 	int callcompletedinsl;
 	struct ao2_iterator memi;
-	struct ast_datastore *datastore = ast_channel_datastore_find(qe->chan, &dialed_interface_info, NULL);
+	struct ast_datastore *datastore;
+
+	ast_channel_lock(qe->chan);
+	datastore = ast_channel_datastore_find(qe->chan, &dialed_interface_info, NULL);
+	ast_channel_unlock(qe->chan);
 
 	memset(&bridge_config, 0, sizeof(bridge_config));
 	time(&now);
@@ -2559,7 +2563,6 @@
 	while ((cur = ao2_iterator_next(&memi))) {
 		struct callattempt *tmp = ast_calloc(1, sizeof(*tmp));
 		struct ast_dialed_interface *di;
-		int dialed = 0;
 		AST_LIST_HEAD(, ast_dialed_interface) *dialed_interfaces;
 		if (!tmp) {
 			ao2_ref(cur, -1);
@@ -2569,48 +2572,67 @@
 			goto out;
 		}
 		if (!datastore) {
-			if(!(datastore = ast_channel_datastore_alloc(&dialed_interface_info, NULL))) {
+			if (!(datastore = ast_channel_datastore_alloc(&dialed_interface_info, NULL))) {
 				ao2_ref(cur, -1);
 				ast_mutex_unlock(&qe->parent->lock);
-				if(use_weight)
+				if (use_weight)
 					AST_LIST_UNLOCK(&queues);
 				free(tmp);
 				goto out;
 			}
 			datastore->inheritance = DATASTORE_INHERIT_FOREVER;
-			dialed_interfaces = ast_calloc(1, sizeof(*dialed_interfaces));
-			datastore->data = dialed_interfaces;
-			AST_LIST_HEAD_INIT(dialed_interfaces);
-			ast_channel_datastore_add(qe->chan, datastore);
-		} else
-			dialed_interfaces = datastore->data;
-		AST_LIST_LOCK(dialed_interfaces);
-		AST_LIST_TRAVERSE(dialed_interfaces, di, list) {
-			/* XXX case sensitive ?? */
-			if(!strcasecmp(cur->interface, di->interface)) {
-				dialed = 1;
-				break;
-			}
-		}
-		if (!dialed && strncasecmp(cur->interface, "Local/", 6)) {
-			if(!(di = ast_calloc(1, sizeof(*di) + strlen(cur->interface)))) {
+			if (!(dialed_interfaces = ast_calloc(1, sizeof(*dialed_interfaces)))) {
 				ao2_ref(cur, -1);
-				AST_LIST_UNLOCK(dialed_interfaces);
 				ast_mutex_unlock(&qe->parent->lock);
-				if(use_weight)
+				if (use_weight)
 					AST_LIST_UNLOCK(&queues);
 				free(tmp);
 				goto out;
 			}
-			strcpy(di->interface, cur->interface);
-			AST_LIST_INSERT_TAIL(dialed_interfaces, di, list);
-		} else if (dialed) {
-			ast_log(LOG_DEBUG, "Skipping dialing interface '%s' since it has already been dialed\n", di->interface);
-			AST_LIST_UNLOCK(dialed_interfaces);
+			datastore->data = dialed_interfaces;
+			AST_LIST_HEAD_INIT(dialed_interfaces);
+
+			ast_channel_lock(qe->chan);
+			ast_channel_datastore_add(qe->chan, datastore);
+			ast_channel_unlock(qe->chan);
+		} else
+			dialed_interfaces = datastore->data;
+
+		AST_LIST_LOCK(dialed_interfaces);
+		AST_LIST_TRAVERSE(dialed_interfaces, di, list) {
+			if (!strcasecmp(cur->interface, di->interface)) {
+				ast_log(LOG_DEBUG, "Skipping dialing interface '%s' since it has already been dialed\n", 
+					di->interface);
+				break;
+			}
+		}
+		AST_LIST_UNLOCK(dialed_interfaces);
+		
+		if (di) {
 			free(tmp);
 			continue;
 		}
-		AST_LIST_UNLOCK(dialed_interfaces);
+
+		/* It is always ok to dial a Local interface.  We only keep track of
+		 * which "real" interfaces have been dialed.  The Local channel will
+		 * inherit this list so that if it ends up dialing a real interface,
+		 * it won't call one that has already been called. */
+		if (strncasecmp(cur->interface, "Local/", 6)) {
+			if (!(di = ast_calloc(1, sizeof(*di) + strlen(cur->interface)))) {
+				ao2_ref(cur, -1);
+				ast_mutex_unlock(&qe->parent->lock);
+				if (use_weight)
+					AST_LIST_UNLOCK(&queues);
+				free(tmp);
+				goto out;
+			}
+			strcpy(di->interface, cur->interface);
+
+			AST_LIST_LOCK(dialed_interfaces);
+			AST_LIST_INSERT_TAIL(dialed_interfaces, di, list);
+			AST_LIST_UNLOCK(dialed_interfaces);
+		}
+
 		tmp->stillgoing = -1;
 		tmp->member = cur;
 		tmp->oldstatus = cur->status;