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

[asterisk-commits at lists.digium.com]

Author: rizzo
Date: Thu Oct 19 02:43:43 2006
New Revision: 45648

URL: http://svn.digium.com/view/asterisk?rev=45648&view=rev
Log:
more documentation of data structure and functions.
Of interest:
+ ast_get_manager_by_name_locked() is now without the ast_
  prefix as it is a local function;

+ unuse_eventqent() renamed to unref_event(), and returns
  the pointer to the next entry.

+ marked with XXX a couple of usages of unref_event()
  because i suspect we are addressing the wrong entry.


Modified:
    trunk/main/manager.c

Modified: trunk/main/manager.c
URL: http://svn.digium.com/view/asterisk/trunk/main/manager.c?rev=45648&r1=45647&r2=45648&view=diff
==============================================================================
--- trunk/main/manager.c (original)
+++ trunk/main/manager.c Thu Oct 19 02:43:43 2006
@@ -77,11 +77,16 @@
 #include "asterisk/linkedlists.h"
 
 struct eventqent {
-	int usecount;
+	int usecount;		/* # of clients who still need the event */
 	int category;
 	struct eventqent *next;
 	char eventdata[1];	/* really variable size, allocated by append_event() */
 };
+struct eventqent *master_eventq = NULL; /* Protected by the sessions list lock */
+/*
+ * XXX for some unclear reasons, we make sure master_eventq always
+ * has one event in it (Placeholder) in init_manager().
+ */
 
 static int enabled = 0;
 static int portno = DEFAULT_MANAGER_PORT;
@@ -93,13 +98,6 @@
 static pthread_t accept_thread_ptr;	/* the accept thread */
 static int block_sockets = 0;
 static int num_sessions = 0;
-
-/* Protected by the sessions list lock */
-struct eventqent *master_eventq = NULL;
-/*
- * XXX for some unclear reasons, we make sure master_eventq always
- * has one event in it (Placeholder) in init_manager().
- */
 
 AST_THREADSTORAGE(manager_event_buf);
 #define MANAGER_EVENT_BUF_INITSIZE   256
@@ -151,8 +149,8 @@
 	char *permit;
 	char *read;
 	char *write;
-	unsigned int displayconnects:1;
-	int keep;
+	int displayconnects;	/* XXX unused */
+	int keep;	/* mark entries created on a reload */
 	AST_LIST_ENTRY(ast_manager_user) list;
 };
 
@@ -287,8 +285,11 @@
 	return ret;
 }
 
-
-static struct ast_manager_user *ast_get_manager_by_name_locked(const char *name)
+/*
+ * lookup an entry in the list of registered users.
+ * must be called with the list lock held.
+ */
+static struct ast_manager_user *get_manager_by_name_locked(const char *name)
 {
 	struct ast_manager_user *user = NULL;
 
@@ -333,7 +334,7 @@
 
 	AST_LIST_LOCK(&users);
 
-	if (!(user = ast_get_manager_by_name_locked(argv[3]))) {
+	if (!(user = get_manager_by_name_locked(argv[3]))) {
 		ast_cli(fd, "There is no manager called %s\n", argv[3]);
 		AST_LIST_UNLOCK(&users);
 		return -1;
@@ -499,25 +500,32 @@
 	showmanager_help, NULL, NULL },
 };
 
-static void unuse_eventqent(struct eventqent *e)
-{
-	if (ast_atomic_dec_and_test(&e->usecount) && e->next)
+/*
+ * Decrement the usecount for the event; if it goes to zero,
+ * (why check for e->next ?) wakeup the
+ * main thread, which is in charge of freeing the record.
+ * Returns the next record.
+ * XXX Locking assumptions ??? next may change if we are last.
+ */
+static struct eventqent *unref_event(struct eventqent *e)
+{
+	struct eventqent *ret = e->next;
+	if (ast_atomic_dec_and_test(&e->usecount) && ret)
 		pthread_kill(accept_thread_ptr, SIGURG);
+	return ret;
 }
 
 static void free_session(struct mansession *s)
 {
-	struct eventqent *eqe;
+	struct eventqent *eqe = s->eventq;
 	if (s->fd > -1)
 		close(s->fd);
 	if (s->outputstr)
 		free(s->outputstr);
 	ast_mutex_destroy(&s->__lock);
-	while ( (eqe = s->eventq) ) {
-		s->eventq = eqe->next;
-		unuse_eventqent(eqe);
-	}
 	free(s);
+	while ( eqe )
+		eqe = unref_event(eqe);
 }
 
 static void destroy_session(struct mansession *s)
@@ -692,7 +700,7 @@
     {
 	/*
 	 * XXX there should be no need to scan the config file again here,
-	 * suffices to call ast_get_manager_by_name_locked() to fetch
+	 * suffices to call get_manager_by_name_locked() to fetch
 	 * the user's entry.
 	 */
 	struct ast_config *cfg = ast_config_load("manager.conf");
@@ -1002,7 +1010,7 @@
 			    ((s->send_events & eqe->category) == eqe->category)) {
 				astman_append(s, "%s", eqe->eventdata);
 			}
-			unuse_eventqent(s->eventq);
+			unref_event(s->eventq);	/* XXX why not eqe ? */
 			s->eventq = eqe;
 		}
 		astman_append(s,
@@ -1713,7 +1721,7 @@
 				if (!ret && ast_carefulwrite(s->fd, eqe->eventdata, strlen(eqe->eventdata), s->writetimeout) < 0)
 					ret = -1;
 			}
-			unuse_eventqent(s->eventq);
+			unref_event(s->eventq);	/* XXX why not eqe ? */
 			s->eventq = eqe;
 		}
 	}
@@ -2014,6 +2022,7 @@
 /*
  * events are appended to a queue from where they
  * can be dispatched to clients.
+ * Must be called with the session lock held (or equivalent).
  */
 static int append_event(const char *str, int category)
 {
@@ -2722,7 +2731,7 @@
 			continue;
 
 		/* Look for an existing entry, if none found - create one and add it to the list */
-		if (!(user = ast_get_manager_by_name_locked(cat))) {
+		if (!(user = get_manager_by_name_locked(cat))) {
 			if (!(user = ast_calloc(1, sizeof(*user))))
 				break;
 			/* Copy name over */
@@ -2768,7 +2777,7 @@
 
 	/* Perform cleanup - essentially prune out old users that no longer exist */
 	AST_LIST_TRAVERSE_SAFE_BEGIN(&users, user, list) {
-		if (user->keep) {
+		if (user->keep) {	/* valid record. clear flag for the next round */
 			user->keep = 0;
 			continue;
 		}