[asterisk-commits] rizzo: trunk r45177 - /trunk/main/manager.c

asterisk-commits at lists.digium.com asterisk-commits at lists.digium.com
Mon Oct 16 02:33:01 MST 2006


Author: rizzo
Date: Mon Oct 16 04:33:00 2006
New Revision: 45177

URL: http://svn.digium.com/view/asterisk?rev=45177&view=rev
Log:
protect access to first_action with actionlock.
Mark with XXX one place (during command execution) where
navigation should be protected with actionlock, but is not
because it would block requests for a long time.

To solve this properly we need to put reference counts in
the struct manager_action.
A suboptimal fix is to copy the record on a search and then
unlock the list while we work on the copy.


Modified:
    trunk/main/manager.c

Modified: trunk/main/manager.c
URL: http://svn.digium.com/view/asterisk/trunk/main/manager.c?rev=45177&r1=45176&r2=45177&view=diff
==============================================================================
--- trunk/main/manager.c (original)
+++ trunk/main/manager.c Mon Oct 16 04:33:00 2006
@@ -220,12 +220,12 @@
 static char *complete_show_mancmd(const char *line, const char *word, int pos, int state)
 {
 	struct manager_action *cur;
-	int which = 0;
+	int l = strlen(word), which = 0;
 	char *ret = NULL;
 
 	ast_mutex_lock(&actionlock);
 	for (cur = first_action; cur; cur = cur->next) { /* Walk the list of actions */
-		if (!strncasecmp(word, cur->action, strlen(word)) && ++which > state) {
+		if (!strncasecmp(word, cur->action, l) && ++which > state) {
 			ret = ast_strdup(cur->action);
 			break;	/* make sure we exit even if ast_strdup() returns NULL */
 		}
@@ -430,7 +430,7 @@
 
 static int handle_showmancmd(int fd, int argc, char *argv[])
 {
-	struct manager_action *cur = first_action;
+	struct manager_action *cur;
 	char authority[80];
 	int num;
 
@@ -438,7 +438,7 @@
 		return RESULT_SHOWUSAGE;
 
 	ast_mutex_lock(&actionlock);
-	for (; cur; cur = cur->next) { /* Walk the list of actions */
+	for (cur = first_action; cur; cur = cur->next) { /* Walk the list of actions */
 		for (num = 3; num < argc; num++) {
 			if (!strcasecmp(cur->action, argv[num])) {
 				ast_cli(fd, "Action: %s\nSynopsis: %s\nPrivilege: %s\n%s\n", cur->action, cur->synopsis, authority_to_str(cur->authority, authority, sizeof(authority) -1), cur->description ? cur->description : "");
@@ -525,7 +525,7 @@
 	Should change to "manager show commands" */
 static int handle_showmancmds(int fd, int argc, char *argv[])
 {
-	struct manager_action *cur = first_action;
+	struct manager_action *cur;
 	char authority[80];
 	char *format = "  %-15.15s  %-15.15s  %-55.55s\n";
 
@@ -533,7 +533,7 @@
 	ast_cli(fd, format, "------", "---------", "--------");
 	
 	ast_mutex_lock(&actionlock);
-	for (; cur; cur = cur->next) /* Walk the list of actions */
+	for (cur = first_action; cur; cur = cur->next) /* Walk the list of actions */
 		ast_cli(fd, format, cur->action, authority_to_str(cur->authority, authority, sizeof(authority) -1), cur->synopsis);
 	ast_mutex_unlock(&actionlock);
 	
@@ -1202,7 +1202,7 @@
 
 static int action_listcommands(struct mansession *s, struct message *m)
 {
-	struct manager_action *cur = first_action;
+	struct manager_action *cur;
 	char idText[256] = "";
 	char temp[BUFSIZ];
 	char *id = astman_get_header(m,"ActionID");
@@ -1211,10 +1211,9 @@
 		snprintf(idText, sizeof(idText), "ActionID: %s\r\n", id);
 	astman_append(s, "Response: Success\r\n%s", idText);
 	ast_mutex_lock(&actionlock);
-	while (cur) { /* Walk the list of actions */
+	for (cur = first_action; cur; cur = cur->next) { /* Walk the list of actions */
 		if ((s->writeperm & cur->authority) == cur->authority)
 			astman_append(s, "%s: %s (Priv: %s)\r\n", cur->action, cur->synopsis, authority_to_str(cur->authority, temp, sizeof(temp)));
-		cur = cur->next;
 	}
 	ast_mutex_unlock(&actionlock);
 	astman_append(s, "\r\n");
@@ -1896,7 +1895,6 @@
 static int process_message(struct mansession *s, struct message *m)
 {
 	char action[80] = "";
-	struct manager_action *tmp = first_action;
 	char *id = astman_get_header(m,"ActionID");
 	char idText[256] = "";
 	int ret = 0;
@@ -1951,10 +1949,12 @@
 		} else
 			astman_send_error(s, m, "Authentication Required");
 	} else {
+		struct manager_action *tmp;
 		ast_mutex_lock(&s->__lock);
 		s->busy++;
 		ast_mutex_unlock(&s->__lock);
-		while (tmp) { 		
+		/* XXX should we protect the list navigation ? */
+		for (tmp = first_action ; tmp; tmp = tmp->next) { 		
 			if (!strcasecmp(action, tmp->action)) {
 				if ((s->writeperm & tmp->authority) == tmp->authority) {
 					if (tmp->func(s, m))
@@ -1964,7 +1964,6 @@
 				}
 				break;
 			}
-			tmp = tmp->next;
 		}
 		if (!tmp)
 			astman_send_error(s, m, "Invalid/unknown command");
@@ -2253,17 +2252,17 @@
 	struct manager_action *cur = first_action, *prev = first_action;
 
 	ast_mutex_lock(&actionlock);
-	while (cur) {
+	for (cur = first_action, prev = NULL; cur; prev = cur, cur = cur->next) {
 		if (!strcasecmp(action, cur->action)) {
-			prev->next = cur->next;
+			if (prev)
+				prev->next = cur->next;
+			else
+				first_action = cur->next;
 			free(cur);
 			if (option_verbose > 1) 
 				ast_verbose(VERBOSE_PREFIX_2 "Manager unregistered action %s\n", action);
-			ast_mutex_unlock(&actionlock);
-			return 0;
-		}
-		prev = cur;
-		cur = cur->next;
+			break;
+		}
 	}
 	ast_mutex_unlock(&actionlock);
 	return 0;
@@ -2278,38 +2277,25 @@
 
 static int ast_manager_register_struct(struct manager_action *act)
 {
-	struct manager_action *cur = first_action, *prev = NULL;
+	struct manager_action *cur, *prev = NULL;
 	int ret;
 
 	ast_mutex_lock(&actionlock);
-	while (cur) { /* Walk the list of actions */
+	for (cur = first_action; cur; prev = cur, cur = cur->next) {
 		ret = strcasecmp(cur->action, act->action);
 		if (ret == 0) {
 			ast_log(LOG_WARNING, "Manager: Action '%s' already registered\n", act->action);
 			ast_mutex_unlock(&actionlock);
 			return -1;
-		} else if (ret > 0) {
-			/* Insert these alphabetically */
-			if (prev) {
-				act->next = prev->next;
-				prev->next = act;
-			} else {
-				act->next = first_action;
-				first_action = act;
-			}
+		}
+		if (ret > 0)	/* Insert these alphabetically */
 			break;
-		}
-		prev = cur; 
-		cur = cur->next;
-	}
-	
-	if (!cur) {
-		if (prev)
-			prev->next = act;
-		else
-			first_action = act;
-		act->next = NULL;
-	}
+	}
+	if (prev)
+		prev->next = act;
+	else
+		first_action = act;
+	act->next = cur;
 
 	if (option_verbose > 1) 
 		ast_verbose(VERBOSE_PREFIX_2 "Manager registered action %s\n", act->action);
@@ -2394,6 +2380,7 @@
 	for (v = params; v; v = v->next) {
 		if (!strcasecmp(v->name, "mansession_id")) {
 			sscanf(v->value, "%lx", &ident);
+			ast_verbose("session is <%lx>\n", ident);
 			break;
 		}
 	}



More information about the asterisk-commits mailing list