[asterisk-commits] mmichelson: branch mmichelson/more_transfer r388347 - in /team/mmichelson/mor...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri May 10 11:51:37 CDT 2013


Author: mmichelson
Date: Fri May 10 11:51:35 2013
New Revision: 388347

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=388347
Log:
Address part of Richard's review feedback.


Modified:
    team/mmichelson/more_transfer/bridges/bridge_builtin_features.c
    team/mmichelson/more_transfer/include/asterisk/channel.h
    team/mmichelson/more_transfer/main/bridging.c
    team/mmichelson/more_transfer/main/channel.c
    team/mmichelson/more_transfer/main/features.c
    team/mmichelson/more_transfer/main/pbx.c

Modified: team/mmichelson/more_transfer/bridges/bridge_builtin_features.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/more_transfer/bridges/bridge_builtin_features.c?view=diff&rev=388347&r1=388346&r2=388347
==============================================================================
--- team/mmichelson/more_transfer/bridges/bridge_builtin_features.c (original)
+++ team/mmichelson/more_transfer/bridges/bridge_builtin_features.c Fri May 10 11:51:35 2013
@@ -94,7 +94,7 @@
 	return res;
 }
 
-static void copy_caller_data(struct ast_channel *caller, struct ast_channel *dest)
+static void copy_caller_data(struct ast_channel *dest, struct ast_channel *caller)
 {
 	ast_channel_lock_both(caller, dest);
 	ast_connected_line_copy_from_caller(ast_channel_connected(dest), ast_channel_caller(caller));
@@ -122,7 +122,7 @@
 	}
 
 	/* Before we actually dial out let's inherit appropriate information. */
-	copy_caller_data(caller, chan);
+	copy_caller_data(chan, caller);
 
 	/* Since the above worked fine now we actually call it and return the channel */
 	if (ast_call(chan, destination, 0)) {
@@ -167,7 +167,7 @@
 {
 	struct ast_channel *transferer_channel = user_data;
 
-	copy_caller_data(transferer_channel, new_channel);
+	copy_caller_data(new_channel, transferer_channel);
 }
 
 /*! \brief Internal built in feature for blind transfers */

Modified: team/mmichelson/more_transfer/include/asterisk/channel.h
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/more_transfer/include/asterisk/channel.h?view=diff&rev=388347&r1=388346&r2=388347
==============================================================================
--- team/mmichelson/more_transfer/include/asterisk/channel.h (original)
+++ team/mmichelson/more_transfer/include/asterisk/channel.h Fri May 10 11:51:35 2013
@@ -4216,9 +4216,11 @@
  * callback
  *
  * XXX Put name of callback-setting function in above paragraph once it is written
- * 
+ *
  * \note When this function returns successfully, the yankee channel is in a state where
  * it cannot be used any further. Always use the returned channel instead.
+ *
+ * \note absolutely _NO_ channel locks should be held before calling this function.
  *
  * \param yankee The channel to gain control of
  * \retval NULL Could not gain control of the channel
@@ -4235,11 +4237,14 @@
  * destination channel will now be running on the source channel instead.
  *
  * \note This function is NOT intended to be used on bridged channels. If you
- * wish to move a channel into a bridge, you can use ast_bridge_impart() and
- * ast_bridge_join() and swap a channel out if desired.
+ * wish to move an unbridged channel into the place of a bridged channel, then
+ * use ast_bridge_join() or ast_bridge_impart(). If you wish to move a bridged
+ * channel into the place of another bridged channel, then use ast_bridge_move().
  *
  * \note When this function returns succesfully, the source channel is in a
  * state where its continued use is unreliable.
+ *
+ * \note absolutely _NO_ channel locks should be held before calling this function.
  *
  * \param dest The place to move the source channel
  * \param source The channel to move

Modified: team/mmichelson/more_transfer/main/bridging.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/more_transfer/main/bridging.c?view=diff&rev=388347&r1=388346&r2=388347
==============================================================================
--- team/mmichelson/more_transfer/main/bridging.c (original)
+++ team/mmichelson/more_transfer/main/bridging.c Fri May 10 11:51:35 2013
@@ -1956,13 +1956,13 @@
 }
 
 static void bridge_channel_attended_transfer(struct ast_bridge_channel *bridge_channel,
-		const char *unbridged_chan_name)
-{
-	RAII_VAR(struct ast_channel *, chan_unbridged, NULL, ao2_cleanup);
+		const char *target_chan_name)
+{
+	RAII_VAR(struct ast_channel *, chan_target, NULL, ao2_cleanup);
 	RAII_VAR(struct ast_channel *, chan_bridged, NULL, ao2_cleanup);
 
-	chan_unbridged = ast_channel_get_by_name(unbridged_chan_name);
-	if (!chan_unbridged) {
+	chan_target = ast_channel_get_by_name(target_chan_name);
+	if (!chan_target) {
 		/* Dang, it disappeared somehow */
 		return;
 	}
@@ -1975,8 +1975,9 @@
 		}
 		ao2_ref(chan_bridged, +1);
 	}
-	
-	ast_channel_move(chan_unbridged, chan_bridged);
+
+/* BUGBUG The ast_channel_move should be performed in an after bridge callback */
+	ast_channel_move(chan_target, chan_bridged);
 }
 
 /*!
@@ -4928,13 +4929,12 @@
 	RAII_VAR(struct ast_bridge_channel *, transferee_bridge_channel, NULL, ao2_cleanup);
 	char unbridged_chan_name[AST_CHANNEL_NAME];
 
-	{
-		SCOPED_LOCK(lock, transferee, ast_channel_lock, ast_channel_unlock);
-		transferee_bridge_channel = ast_channel_internal_bridge_channel(transferee);
-		if (!transferee_bridge_channel) {
-			return -1;
-		}
-		ao2_ref(transferee_bridge_channel, +1);
+	ast_channel_lock(transferee);
+	transferee_bridge_channel = ast_channel_get_bridge_channel(transferee);
+	ast_channel_unlock(transferee);
+
+	if (!transferee_bridge_channel) {
+		return -1;
 	}
 
 	ast_copy_string(unbridged_chan_name, ast_channel_name(unbridged_chan),

Modified: team/mmichelson/more_transfer/main/channel.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/more_transfer/main/channel.c?view=diff&rev=388347&r1=388346&r2=388347
==============================================================================
--- team/mmichelson/more_transfer/main/channel.c (original)
+++ team/mmichelson/more_transfer/main/channel.c Fri May 10 11:51:35 2013
@@ -11313,7 +11313,6 @@
 		char *context;
 		char *linkedid;
 		char *name;
-		struct ast_cdr *cdr;
 		int amaflags;
 		int state;
 		struct ast_format readformat;
@@ -11327,26 +11326,16 @@
 	tmpvars.linkedid = ast_strdupa(ast_channel_linkedid(yankee));
 	tmpvars.name = ast_strdupa(ast_channel_name(yankee));
 	tmpvars.amaflags = ast_channel_amaflags(yankee);
-	tmpvars.state = ast_channel_state(yankee);
 	ast_format_copy(&tmpvars.writeformat, ast_channel_writeformat(yankee));
 	ast_format_copy(&tmpvars.readformat, ast_channel_readformat(yankee));
-	tmpvars.cdr = ast_channel_cdr(yankee) ? ast_cdr_dup(ast_channel_cdr(yankee)) : NULL;
 	ast_channel_unlock(yankee);
 
 	/* Do not hold any channel locks while calling channel_alloc() since the function
 	 * locks the channel container when linking the new channel in. */
-	if (!(tmpchan = ast_channel_alloc(0, tmpvars.state, 0, 0, tmpvars.accountcode,
+	if (!(tmpchan = ast_channel_alloc(0, AST_STATE_DOWN, 0, 0, tmpvars.accountcode,
 					tmpvars.exten, tmpvars.context, tmpvars.linkedid, tmpvars.amaflags,
 					"Surrogate/%s", tmpvars.name))) {
-		ast_cdr_discard(tmpvars.cdr);
 		return NULL;
-	}
-
-	/* copy the cdr info over */
-	if (tmpvars.cdr) {
-		ast_cdr_discard(ast_channel_cdr(tmpchan));
-		ast_channel_cdr_set(tmpchan, tmpvars.cdr);
-		tmpvars.cdr = NULL;
 	}
 
 	/* Make formats okay */

Modified: team/mmichelson/more_transfer/main/features.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/more_transfer/main/features.c?view=diff&rev=388347&r1=388346&r2=388347
==============================================================================
--- team/mmichelson/more_transfer/main/features.c (original)
+++ team/mmichelson/more_transfer/main/features.c Fri May 10 11:51:35 2013
@@ -7171,13 +7171,7 @@
 	ast_channel_unlock(chan);
 
 	if (chan_bridge) {
-		/* XXX ast_bridge_move() does not take an ast_bridge_features
-		 * parameter. I could presumably get the channel's bridge_channel
-		 * and substitute the features directly onto the bridge_channel.
-		 * However, because of inconsistencies with allocation of features,
-		 * I can't know whether I need to free the old bridge features or
-		 * if attempting to free the bridge features will cause a crash.
-		 */
+/* BUGBUG The supplied features are not applied in this case */
 		if (ast_bridge_move(bridge, chan_bridge, chan, NULL, 1)) {
 			return -1;
 		}
@@ -7210,9 +7204,9 @@
 		if (!play_bridge_channel) {
 			ast_log(LOG_WARNING, "Unable to play tone for channel %s. Unable to get bridge channel\n",
 					ast_channel_name(play_chan));
-		}
-		
-		ast_bridge_channel_queue_playfile(play_bridge_channel, NULL, xfersound, NULL);
+		} else {
+			ast_bridge_channel_queue_playfile(play_bridge_channel, NULL, xfersound, NULL);
+		}
 	}
 	return 0;
 }
@@ -7291,6 +7285,7 @@
 	if (add_to_bridge(bridge, chana, NULL, ast_true(playtone))) {
 		snprintf(buf, sizeof(buf), "Unable to add Channel1 to bridge: %s", ast_channel_name(chana));
 		astman_send_error(s, m, buf);
+		ast_bridge_destroy(bridge);
 		return 0;
 	}
 
@@ -7298,6 +7293,7 @@
 	if (add_to_bridge(bridge, chanb, NULL, ast_true(playtone))) {
 		snprintf(buf, sizeof(buf), "Unable to add Channel2 to bridge: %s", ast_channel_name(chanb));
 		astman_send_error(s, m, buf);
+		ast_bridge_destroy(bridge);
 		return 0;
 	}
 
@@ -8170,8 +8166,7 @@
 		goto done;
 	}
 
-	if (add_to_bridge(bridge, current_dest_chan, peer_features, ast_test_flag(&opts, BRIDGE_OPT_PLAYTONE)))
-	{
+	if (add_to_bridge(bridge, current_dest_chan, peer_features, ast_test_flag(&opts, BRIDGE_OPT_PLAYTONE))) {
 		ast_bridge_features_destroy(peer_features);
 		ast_bridge_features_cleanup(&chan_features);
 		ast_bridge_destroy(bridge);

Modified: team/mmichelson/more_transfer/main/pbx.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/more_transfer/main/pbx.c?view=diff&rev=388347&r1=388346&r2=388347
==============================================================================
--- team/mmichelson/more_transfer/main/pbx.c (original)
+++ team/mmichelson/more_transfer/main/pbx.c Fri May 10 11:51:35 2013
@@ -9391,9 +9391,14 @@
 		ast_channel_unlock(chan);
 		return 0;
 	}
+	ast_channel_unlock(chan);
 
 	/* Otherwise, we need to gain control of the channel first */
 	newchan = ast_channel_yank(chan);
+	if (!newchan) {
+		ast_log(LOG_WARNING, "Unable to gain control of channel %s\n", ast_channel_name(chan));
+		return -1;
+	}
 	ast_explicit_goto(newchan, context, exten, priority);
 	if (ast_pbx_start(newchan)) {
 		ast_hangup(newchan);




More information about the asterisk-commits mailing list