[asterisk-commits] russell: branch russell/messaging r309631 - in /team/russell/messaging: inclu...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Mar 4 16:06:55 CST 2011


Author: russell
Date: Fri Mar  4 16:06:50 2011
New Revision: 309631

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=309631
Log:
some fixes and cleanup

Modified:
    team/russell/messaging/include/asterisk/message.h
    team/russell/messaging/main/message.c

Modified: team/russell/messaging/include/asterisk/message.h
URL: http://svnview.digium.com/svn/asterisk/team/russell/messaging/include/asterisk/message.h?view=diff&rev=309631&r1=309630&r2=309631
==============================================================================
--- team/russell/messaging/include/asterisk/message.h (original)
+++ team/russell/messaging/include/asterisk/message.h Fri Mar  4 16:06:50 2011
@@ -60,6 +60,16 @@
         const char * const name;
         /*!
          * \brief Send a message.
+	 *
+	 * \param msg the message to send
+	 * \param to the URI of where the message is being sent
+	 * \param from the URI of where the message was sent from
+	 *
+	 * The fields of the ast_msg are guaranteed not to change during the
+	 * duration of this function call.
+	 *
+	 * \retval 0 success
+	 * \retval non-zero failure
          */
         int (* const msg_send)(const struct ast_msg *msg, const char *to, const char *from);
 };
@@ -70,21 +80,7 @@
  * \retval 0 success
  * \retval non-zero failure
  */
-#define ast_msg_tech_register(t) __ast_msg_tech_register((t), ast_module_info->self)
-
-/*!
- * \brief Register a message technology
- *
- * In general, this function should not be used directly.  It should be used through
- * the ast_msg_tech_register() wrapper.  The only case where this should be used
- * directly is if something inside of the Asterisk core (as opposed to a loadable
- * module) was registering a message technology.  In that case, this function would
- * be used directly and the ast_module argument would be NULL.
- *
- * \retval 0 success
- * \retval non-zero failure
- */
-int __ast_msg_tech_register(const struct ast_msg_tech *tech, struct ast_module *mod);
+int ast_msg_tech_register(const struct ast_msg_tech *tech);
 
 /*!
  * \brief Unregister a message technology.

Modified: team/russell/messaging/main/message.c
URL: http://svnview.digium.com/svn/asterisk/team/russell/messaging/main/message.c?view=diff&rev=309631&r1=309630&r2=309631
==============================================================================
--- team/russell/messaging/main/message.c (original)
+++ team/russell/messaging/main/message.c Fri Mar  4 16:06:50 2011
@@ -29,11 +29,11 @@
 
 #include "asterisk/_private.h"
 
+#include "asterisk/module.h"
 #include "asterisk/datastore.h"
 #include "asterisk/pbx.h"
 #include "asterisk/strings.h"
 #include "asterisk/astobj2.h"
-#include "asterisk/module.h"
 #include "asterisk/app.h"
 #include "asterisk/taskprocessor.h"
 #include "asterisk/message.h"
@@ -41,7 +41,7 @@
 /*!
  * \brief A message.
  *
- * XXX \todo Optimize string handling with stringfields as appropriate.
+ * \todo Consider whether stringfields would be an appropriate optimization here.
  */
 struct ast_msg {
 	struct ast_str *to;
@@ -52,7 +52,6 @@
 
 struct ast_msg_tech_holder {
 	const struct ast_msg_tech *tech;
-	struct ast_module *mod;
 };
 
 static struct ao2_container *msg_techs;
@@ -88,6 +87,7 @@
 		unsigned int duration);
 
 /*!
+ * \internal
  * \brief A bare minimum channel technology
  *
  * This will not be registered as we never want anything to try
@@ -343,20 +343,43 @@
  */
 static void chan_cleanup(struct ast_channel *chan)
 {
-	struct ast_datastore *ds;
+	struct ast_datastore *msg_ds, *ds;
+	struct varshead *headp;
+	struct ast_var_t *vardata;
 
 	ast_channel_lock(chan);
 
-	if ((ds = ast_channel_datastore_find(chan, &msg_datastore, NULL)) && ds->data) {
-		ao2_ref(ds->data, -1);
-		ds->data = NULL;
-	}
-
 	/*
-	 * XXX Other cleanup to do:
-	 *  - destroy all non-msg datastores
-	 *  - destroy all channel variables
+	 * Remove the msg datastore.  Free its data but keep around the datastore
+	 * object and just reuse it.
 	 */
+	if ((msg_ds = ast_channel_datastore_find(chan, &msg_datastore, NULL)) && msg_ds->data) {
+		ast_channel_datastore_remove(chan, msg_ds);
+		ao2_ref(msg_ds->data, -1);
+		msg_ds->data = NULL;
+	}
+
+	/*
+	 * Destroy all other datastores.
+	 */
+	while ((ds = AST_LIST_REMOVE_HEAD(&chan->datastores, entry))) {
+		ast_datastore_free(ds);
+	}
+
+	/*
+	 * Destroy all channel variables.
+	 */
+	headp = &chan->varshead;
+	while ((vardata = AST_LIST_REMOVE_HEAD(headp, entries))) {
+		ast_var_delete(vardata);
+	}
+
+	/*
+	 * Restore msg datastore.
+	 */
+	if (msg_ds) {
+		ast_channel_datastore_add(chan, msg_ds);
+	}
 
 	ast_channel_unlock(chan);
 }
@@ -518,6 +541,35 @@
 	ao2_ref(msg, -1);
 
 	return 0;
+}
+
+static int msg_tech_hash(const void *obj, const int flags)
+{
+	const struct ast_msg_tech_holder *tech_holder = obj;
+
+	return ast_str_case_hash(tech_holder->tech->name);
+}
+
+static int msg_tech_cmp(void *obj, void *arg, int flags)
+{
+	const struct ast_msg_tech_holder *tech_holder = obj;
+	const struct ast_msg_tech_holder *tech_holder2 = arg;
+
+	return strcasecmp(tech_holder->tech->name, tech_holder2->tech->name) ?  0 : CMP_MATCH | CMP_STOP;
+}
+
+static int msg_tech_cmp_locked(void *obj, void *arg, int flags)
+{
+	int res;
+	struct ast_msg_tech_holder *tech_holder = obj;
+
+	res = msg_tech_cmp(obj, arg, flags);
+
+	if (res == (CMP_MATCH | CMP_STOP)) {
+		ao2_lock(tech_holder);
+	}
+
+	return res;
 }
 
 /*!
@@ -569,10 +621,15 @@
 	tech_name = strsep(&tech_name, ":");
 
 	{
-		struct ast_msg_tech tmp_msg_tech = { .name = tech_name, };
-		struct ast_msg_tech_holder tmp_tech_holder = { .tech = &tmp_msg_tech, };
-
-		tech_holder = ao2_find(msg_techs, &tmp_tech_holder, OBJ_POINTER);
+		struct ast_msg_tech tmp_msg_tech = {
+			.name = tech_name,
+		};
+		struct ast_msg_tech_holder tmp_tech_holder = {
+			.tech = &tmp_msg_tech,
+		};
+
+		tech_holder = ao2_callback(msg_techs, OBJ_POINTER,
+					msg_tech_cmp_locked, &tmp_tech_holder);
 	}
 
 	if (!tech_holder) {
@@ -583,19 +640,18 @@
 
 	/*
 	 * The message lock is held here to safely allow the technology
-	 * implementation access the message fields without worrying
+	 * implementation to access the message fields without worrying
 	 * that they could change.
 	 */
-	ast_module_ref(tech_holder->mod);
 	ao2_lock(msg);
 	res = tech_holder->tech->msg_send(msg, args.to, args.from);
 	ao2_unlock(msg);
-	ast_module_unref(tech_holder->mod);
 
 	pbx_builtin_setvar_helper(chan, "MESSAGE_SEND_STATUS", res ? "FAILURE" : "SUCCESS");
 
 exit_cleanup:
 	if (tech_holder) {
+		ao2_unlock(tech_holder);
 		ao2_ref(tech_holder, -1);
 		tech_holder = NULL;
 	}
@@ -605,7 +661,7 @@
 	return 0;
 }
 
-int __ast_msg_tech_register(const struct ast_msg_tech *tech, struct ast_module *mod)
+int ast_msg_tech_register(const struct ast_msg_tech *tech)
 {
 	struct ast_msg_tech_holder tmp_tech_holder = {
 		.tech = tech,
@@ -624,7 +680,6 @@
 	}
 
 	tech_holder->tech = tech;
-	tech_holder->mod = mod;
 
 	ao2_link(msg_techs, tech_holder);
 
@@ -650,6 +705,16 @@
 		return -1;
 	}
 
+	/*
+	 * The code that sends messages using a message technology ensures that it
+	 * locks the tech_holder before it returns a reference to it from the
+	 * container.  By just grabbing and releasing the lock here, we ensure that
+	 * we wait until any currently running message send code finishes before
+	 * we proceed with unregistration.
+	 */
+	ao2_lock(tech_holder);
+	ao2_unlock(tech_holder);
+
 	ao2_ref(tech_holder, -1);
 	tech_holder = NULL;
 
@@ -658,21 +723,6 @@
 	return 0;
 }
 
-static int msg_tech_hash(const void *obj, const int flags)
-{
-	const struct ast_msg_tech_holder *tech_holder = obj;
-
-	return ast_str_case_hash(tech_holder->tech->name);
-}
-
-static int msg_tech_cmp(void *obj, void *arg, int flags)
-{
-	const struct ast_msg_tech_holder *tech_holder = obj;
-	const struct ast_msg_tech_holder *tech_holder2 = arg;
-
-	return strcasecmp(tech_holder->tech->name, tech_holder2->tech->name) ?  0 : CMP_MATCH | CMP_STOP;
-}
-
 /*
  * \internal
  * \brief Initialize stuff during Asterisk startup.




More information about the asterisk-commits mailing list