Restore Dial, Queue, and FollowMe 'I' option support.

The Dial, Queue, and FollowMe applications need to inhibit the bridging
initial connected line exchange in order to support the 'I' option.

* Replaced the pass_reference flag on ast_bridge_join() with a flags
parameter to pass other flags defined by enum ast_bridge_join_flags.

* Replaced the independent flag on ast_bridge_impart() with a flags
parameter to pass other flags defined by enum ast_bridge_impart_flags.

* Since the Dial, Queue, and FollowMe applications are now the only
callers of ast_bridge_call() and ast_bridge_call_with_flags(), changed the
calling contract to require the initial COLP exchange to already have been
done by the caller.

* Made all callers of ast_bridge_impart() check the return value.  It is
important.  As a precaution, I also made the compiler complain now if it
is not checked.

* Did some cleanup in parking_tests.c as a result of checking the
ast_bridge_impart() return value.

An independent, but associated change is:
* Reduce stack usage in ast_indicate_data() and add a dropping redundant
connected line verbose message.

(closes issue ASTERISK-22072)
Reported by: Joshua Colp

Review: https://reviewboard.asterisk.org/r/2845/
........

Merged revisions 399136 from http://svn.asterisk.org/svn/asterisk/branches/12


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@399138 65c4cc65-6c06-0410-ace0-fbb531ad65f3

7 files changed, 123 insertions, 55 deletions

diff --git a/main/bridge.c b/main/bridge.c
index 45820c3fa..19350ed84 100644
--- a/main/bridge.c
+++ b/main/bridge.c
@@ -1433,13 +1433,13 @@ int ast_bridge_join(struct ast_bridge *bridge,
 	struct ast_channel *swap,
 	struct ast_bridge_features *features,
 	struct ast_bridge_tech_optimizations *tech_args,
-	int pass_reference)
+	enum ast_bridge_join_flags flags)
 {
 	struct ast_bridge_channel *bridge_channel;
 	int res = 0;
 
 	bridge_channel = bridge_channel_internal_alloc(bridge);
-	if (pass_reference) {
+	if (flags & AST_BRIDGE_JOIN_PASS_REFERENCE) {
 		ao2_ref(bridge, -1);
 	}
 	if (!bridge_channel) {
@@ -1468,6 +1468,7 @@ int ast_bridge_join(struct ast_bridge *bridge,
 	bridge_channel->chan = chan;
 	bridge_channel->swap = swap;
 	bridge_channel->features = features;
+	bridge_channel->inhibit_colp = !!(flags & AST_BRIDGE_JOIN_INHIBIT_JOIN_COLP);
 
 	if (!res) {
 		res = bridge_channel_internal_join(bridge_channel);
@@ -1546,7 +1547,11 @@ static void *bridge_channel_ind_thread(void *data)
 	return NULL;
 }
 
-int ast_bridge_impart(struct ast_bridge *bridge, struct ast_channel *chan, struct ast_channel *swap, struct ast_bridge_features *features, int independent)
+int ast_bridge_impart(struct ast_bridge *bridge,
+	struct ast_channel *chan,
+	struct ast_channel *swap,
+	struct ast_bridge_features *features,
+	enum ast_bridge_impart_flags flags)
 {
 	int res = 0;
 	struct ast_bridge_channel *bridge_channel;
@@ -1585,12 +1590,14 @@ int ast_bridge_impart(struct ast_bridge *bridge, struct ast_channel *chan, struc
 	bridge_channel->chan = chan;
 	bridge_channel->swap = swap;
 	bridge_channel->features = features;
-	bridge_channel->depart_wait = independent ? 0 : 1;
+	bridge_channel->inhibit_colp = !!(flags & AST_BRIDGE_IMPART_INHIBIT_JOIN_COLP);
+	bridge_channel->depart_wait =
+		(flags & AST_BRIDGE_IMPART_CHAN_MASK) == AST_BRIDGE_IMPART_CHAN_DEPARTABLE;
 	bridge_channel->callid = ast_read_threadstorage_callid();
 
 	/* Actually create the thread that will handle the channel */
 	if (!res) {
-		if (independent) {
+		if ((flags & AST_BRIDGE_IMPART_CHAN_MASK) == AST_BRIDGE_IMPART_CHAN_INDEPENDENT) {
 			res = ast_pthread_create_detached(&bridge_channel->thread, NULL,
 				bridge_channel_ind_thread, bridge_channel);
 		} else {
@@ -2191,7 +2198,8 @@ int ast_bridge_add_channel(struct ast_bridge *bridge, struct ast_channel *chan,
 			ast_answer(yanked_chan);
 		}
 		ast_channel_ref(yanked_chan);
-		if (ast_bridge_impart(bridge, yanked_chan, NULL, features, 1)) {
+		if (ast_bridge_impart(bridge, yanked_chan, NULL, features,
+			AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
 			/* It is possible for us to yank a channel and have some other
 			 * thread start a PBX on the channl after we yanked it. In particular,
 			 * this can theoretically happen on the ;2 of a Local channel if we
@@ -3638,7 +3646,8 @@ static enum ast_transfer_result blind_transfer_bridge(struct ast_channel *transf
 		ast_hangup(local);
 		return AST_BRIDGE_TRANSFER_FAIL;
 	}
-	if (ast_bridge_impart(bridge, local, transferer, NULL, 1)) {
+	if (ast_bridge_impart(bridge, local, transferer, NULL,
+		AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
 		ast_hangup(local);
 		return AST_BRIDGE_TRANSFER_FAIL;
 	}
@@ -3808,7 +3817,8 @@ static enum ast_transfer_result attended_transfer_bridge(struct ast_channel *cha
 		return AST_BRIDGE_TRANSFER_FAIL;
 	}
 
-	if (ast_bridge_impart(bridge1, local_chan, chan1, NULL, 1)) {
+	if (ast_bridge_impart(bridge1, local_chan, chan1, NULL,
+		AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
 		ast_hangup(local_chan);
 		return AST_BRIDGE_TRANSFER_FAIL;
 	}
diff --git a/main/bridge_basic.c b/main/bridge_basic.c
index 103099d8b..aaa77caea 100644
--- a/main/bridge_basic.c
+++ b/main/bridge_basic.c
@@ -2289,7 +2289,8 @@ static enum attended_transfer_state recalling_exit(struct attended_transfer_prop
 		 */
 		ast_bridge_features_ds_set(props->recall_target, &props->transferer_features);
 		ast_channel_ref(props->recall_target);
-		if (ast_bridge_impart(props->transferee_bridge, props->recall_target, NULL, NULL, 1)) {
+		if (ast_bridge_impart(props->transferee_bridge, props->recall_target, NULL, NULL,
+			AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
 			ast_hangup(props->recall_target);
 			return TRANSFER_FAIL;
 		}
@@ -2380,7 +2381,8 @@ static int retransfer_enter(struct attended_transfer_properties *props)
 	}
 
 	ast_channel_ref(props->recall_target);
-	if (ast_bridge_impart(props->transferee_bridge, props->recall_target, NULL, NULL, 1)) {
+	if (ast_bridge_impart(props->transferee_bridge, props->recall_target, NULL, NULL,
+		AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
 		ast_log(LOG_ERROR, "Unable to place recall target into bridge\n");
 		ast_hangup(props->recall_target);
 		return -1;
@@ -3067,7 +3069,8 @@ static int feature_attended_transfer(struct ast_bridge_channel *bridge_channel,
 	 * choice is to give it a bump
 	 */
 	ast_channel_ref(props->transfer_target);
-	if (ast_bridge_impart(props->target_bridge, props->transfer_target, NULL, NULL, 1)) {
+	if (ast_bridge_impart(props->target_bridge, props->transfer_target, NULL, NULL,
+		AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
 		ast_log(LOG_ERROR, "Unable to place transfer target into bridge.\n");
 		ast_stream_and_wait(bridge_channel->chan, props->failsound, AST_DIGIT_NONE);
 		ast_bridge_channel_write_unhold(bridge_channel);
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 1aa82abfc..2917e83c5 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -2000,7 +2000,7 @@ int bridge_channel_internal_join(struct ast_bridge_channel *bridge_channel)
 			BRIDGE_CHANNEL_STATE_END_NO_DISSOLVE, bridge_channel->bridge->cause);
 		res = -1;
 	}
-	bridge_reconfigured(bridge_channel->bridge, 1);
+	bridge_reconfigured(bridge_channel->bridge, !bridge_channel->inhibit_colp);
 
 	if (bridge_channel->state == BRIDGE_CHANNEL_STATE_WAIT) {
 		/*
diff --git a/main/channel.c b/main/channel.c
index f085d31e8..7ba3e6c39 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -4380,6 +4380,89 @@ const char *ast_channel_amaflags2string(enum ama_flags flag)
 	}
 }
 
+/*!
+ * \internal
+ * \brief Preprocess connected line update.
+ * \since 12.0.0
+ *
+ * \param chan channel to change the indication
+ * \param data pointer to payload data
+ * \param datalen size of payload data
+ *
+ * \note This function assumes chan is locked.
+ *
+ * \retval 0 keep going.
+ * \retval -1 quit now.
+ */
+static int indicate_connected_line(struct ast_channel *chan, const void *data, size_t datalen)
+{
+	struct ast_party_connected_line *chan_connected = ast_channel_connected(chan);
+	struct ast_party_connected_line *chan_indicated = ast_channel_connected_indicated(chan);
+	struct ast_party_connected_line connected;
+	unsigned char current[1024];
+	unsigned char proposed[1024];
+	int current_size;
+	int proposed_size;
+	int res;
+
+	ast_party_connected_line_set_init(&connected, chan_connected);
+	res = ast_connected_line_parse_data(data, datalen, &connected);
+	if (!res) {
+		ast_channel_set_connected_line(chan, &connected, NULL);
+	}
+	ast_party_connected_line_free(&connected);
+	if (res) {
+		return -1;
+	}
+
+	current_size = ast_connected_line_build_data(current, sizeof(current),
+		chan_indicated, NULL);
+	proposed_size = ast_connected_line_build_data(proposed, sizeof(proposed),
+		chan_connected, NULL);
+	if (current_size == -1 || proposed_size == -1) {
+		return -1;
+	}
+
+	if (current_size == proposed_size && !memcmp(current, proposed, current_size)) {
+		ast_debug(1, "%s: Dropping redundant connected line update \"%s\" <%s>.\n",
+			ast_channel_name(chan),
+			S_COR(chan_connected->id.name.valid, chan_connected->id.name.str, ""),
+			S_COR(chan_connected->id.number.valid, chan_connected->id.number.str, ""));
+		return -1;
+	}
+
+	ast_party_connected_line_copy(chan_indicated, chan_connected);
+	return 0;
+}
+
+/*!
+ * \internal
+ * \brief Preprocess redirecting update.
+ * \since 12.0.0
+ *
+ * \param chan channel to change the indication
+ * \param data pointer to payload data
+ * \param datalen size of payload data
+ *
+ * \note This function assumes chan is locked.
+ *
+ * \retval 0 keep going.
+ * \retval -1 quit now.
+ */
+static int indicate_redirecting(struct ast_channel *chan, const void *data, size_t datalen)
+{
+	struct ast_party_redirecting redirecting;
+	int res;
+
+	ast_party_redirecting_set_init(&redirecting, ast_channel_redirecting(chan));
+	res = ast_redirecting_parse_data(data, datalen, &redirecting);
+	if (!res) {
+		ast_channel_set_redirecting(chan, &redirecting, NULL);
+	}
+	ast_party_redirecting_free(&redirecting);
+	return res ? -1 : 0;
+}
+
 int ast_indicate_data(struct ast_channel *chan, int _condition,
 		const void *data, size_t datalen)
 {
@@ -4414,7 +4497,6 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
 		/* who knows what we will get back! the anticipation is killing me. */
 		if (!(awesome_frame = ast_framehook_list_write_event(ast_channel_framehooks(chan), awesome_frame))
 			|| awesome_frame->frametype != AST_FRAME_CONTROL) {
-
 			res = 0;
 			goto indicate_cleanup;
 		}
@@ -4426,46 +4508,15 @@ int ast_indicate_data(struct ast_channel *chan, int _condition,
 
 	switch (condition) {
 	case AST_CONTROL_CONNECTED_LINE:
-		{
-			struct ast_party_connected_line connected;
-			unsigned char current[1024], proposed[1024];
-			int current_size, proposed_size;
-
-			ast_party_connected_line_set_init(&connected, ast_channel_connected(chan));
-			res = ast_connected_line_parse_data(data, datalen, &connected);
-			if (!res) {
-				ast_channel_set_connected_line(chan, &connected, NULL);
-			}
-			ast_party_connected_line_free(&connected);
-
-			current_size = ast_connected_line_build_data(current, sizeof(current),
-				ast_channel_connected_indicated(chan), NULL);
-			proposed_size = ast_connected_line_build_data(proposed, sizeof(proposed),
-				ast_channel_connected(chan), NULL);
-
-			if (current_size == -1 || proposed_size == -1) {
-				goto indicate_cleanup;
-			}
-
-			if (!res && current_size == proposed_size &&
-				!memcmp(current, proposed, current_size)) {
-				goto indicate_cleanup;
-			}
-
-			ast_party_connected_line_copy(ast_channel_connected_indicated(chan),
-				ast_channel_connected(chan));
+		if (indicate_connected_line(chan, data, datalen)) {
+			res = 0;
+			goto indicate_cleanup;
 		}
 		break;
 	case AST_CONTROL_REDIRECTING:
-		{
-			struct ast_party_redirecting redirecting;
-
-			ast_party_redirecting_set_init(&redirecting, ast_channel_redirecting(chan));
-			res = ast_redirecting_parse_data(data, datalen, &redirecting);
-			if (!res) {
-				ast_channel_set_redirecting(chan, &redirecting, NULL);
-			}
-			ast_party_redirecting_free(&redirecting);
+		if (indicate_redirecting(chan, data, datalen)) {
+			res = 0;
+			goto indicate_cleanup;
 		}
 		break;
 	case AST_CONTROL_HOLD:
diff --git a/main/core_local.c b/main/core_local.c
index 88bf3ae4c..d4ec06d9f 100644
--- a/main/core_local.c
+++ b/main/core_local.c
@@ -687,7 +687,7 @@ static int local_call(struct ast_channel *ast, const char *dest, int timeout)
 		publish_local_bridge_message(p);
 		ast_answer(chan);
 		res = ast_bridge_impart(p->action.bridge.join, chan, p->action.bridge.swap,
-			p->action.bridge.features, 1);
+			p->action.bridge.features, AST_BRIDGE_IMPART_CHAN_INDEPENDENT);
 		ao2_ref(p->action.bridge.join, -1);
 		p->action.bridge.join = NULL;
 		ao2_cleanup(p->action.bridge.swap);
diff --git a/main/core_unreal.c b/main/core_unreal.c
index 07748b442..7e457f484 100644
--- a/main/core_unreal.c
+++ b/main/core_unreal.c
@@ -758,7 +758,8 @@ int ast_unreal_channel_push_to_bridge(struct ast_channel *ast, struct ast_bridge
 	ast_set_flag(&features->feature_flags, flags);
 
 	/* Impart the semi2 channel into the bridge */
-	if (ast_bridge_impart(bridge, chan, NULL, features, 1)) {
+	if (ast_bridge_impart(bridge, chan, NULL, features,
+		AST_BRIDGE_IMPART_CHAN_INDEPENDENT)) {
 		ast_bridge_features_destroy(features);
 		ast_channel_unref(chan);
 		return -1;
diff --git a/main/features.c b/main/features.c
index 32fccf7a6..cb4abbb44 100644
--- a/main/features.c
+++ b/main/features.c
@@ -675,7 +675,8 @@ int ast_bridge_call_with_flags(struct ast_channel *chan, struct ast_channel *pee
 	ast_bridge_basic_set_flags(bridge, flags);
 
 	/* Put peer into the bridge */
-	if (ast_bridge_impart(bridge, peer, NULL, peer_features, 1)) {
+	if (ast_bridge_impart(bridge, peer, NULL, peer_features,
+		AST_BRIDGE_IMPART_CHAN_INDEPENDENT | AST_BRIDGE_IMPART_INHIBIT_JOIN_COLP)) {
 		ast_bridge_destroy(bridge, 0);
 		ast_bridge_features_cleanup(&chan_features);
 		bridge_failed_peer_goto(chan, peer);
@@ -683,7 +684,8 @@ int ast_bridge_call_with_flags(struct ast_channel *chan, struct ast_channel *pee
 	}
 
 	/* Join bridge */
-	ast_bridge_join(bridge, chan, NULL, &chan_features, NULL, 1);
+	ast_bridge_join(bridge, chan, NULL, &chan_features, NULL,
+		AST_BRIDGE_JOIN_PASS_REFERENCE | AST_BRIDGE_JOIN_INHIBIT_JOIN_COLP);
 
 	/*
 	 * If the bridge was broken for a hangup that isn't real, then
@@ -1129,7 +1131,8 @@ static int bridge_exec(struct ast_channel *chan, const char *data)
 		goto done;
 	}
 
-	ast_bridge_join(bridge, chan, NULL, &chan_features, NULL, 1);
+	ast_bridge_join(bridge, chan, NULL, &chan_features, NULL,
+		AST_BRIDGE_JOIN_PASS_REFERENCE);
 
 	ast_bridge_features_cleanup(&chan_features);