[Asterisk-code-review] features.c: Make Bridge application tolerate unspecified channel. (asterisk[16])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13513 )

Change subject: features.c: Make Bridge application tolerate unspecified channel.
......................................................................

features.c: Make Bridge application tolerate unspecified channel.

The Bridge application was inconsistent if the channel to bridge with is
not specified.  If no parameters are given then a warning is issued and
the current channel is hung up.  If options are given but no channel is
specified then a warning is issued and the current channel is not hung up.

* Made the Bridge application give a verbose message instead of a warning
if the channel to bridge with is not specified and made not hang up the
current channel.  As a result dialplan no longer needs to check if a
channel name is passed before calling Bridge and simply needs to check the
BRIDGERESULT channel variable instead.  This is something you likely want
your dialplan to do anyway.

* Fixed up L() option warning message.  It is up to the caller to
determine if the channel is hung up because of the warning.  Dial() hangs
up the current channel while Bridge() does not.

Change-Id: I44349a8dc3912397f28852777de04f19e7bb9c73
---
M main/features.c
1 file changed, 15 insertions(+), 15 deletions(-)

Approvals:
  Sean Bright: Looks good to me, but someone else must approve
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/main/features.c b/main/features.c
index 8c2d9bc..ab03bef 100644
--- a/main/features.c
+++ b/main/features.c
@@ -84,7 +84,9 @@
 		</synopsis>
 		<syntax>
 			<parameter name="channel" required="true">
-				<para>The current channel is bridged to the specified <replaceable>channel</replaceable>.</para>
+				<para>The current channel is bridged to the channel
+				identified by the channel name, channel name prefix, or channel
+				uniqueid.</para>
 			</parameter>
 			<parameter name="options">
 				<optionlist>
@@ -893,7 +895,7 @@
 		config->warning_freq = atol(warnfreq_str);
 
 	if (!config->timelimit) {
-		ast_log(LOG_WARNING, "Bridge does not accept L(%s), hanging up.\n", limit_str);
+		ast_log(LOG_WARNING, "Bridge does not accept L(%s)\n", limit_str);
 		config->timelimit = config->play_warning = config->warning_freq = 0;
 		config->warning_sound = NULL;
 		return -1; /* error */
@@ -996,7 +998,7 @@
  */
 static int bridge_exec(struct ast_channel *chan, const char *data)
 {
-	struct ast_channel *current_dest_chan;
+	struct ast_channel *current_dest_chan = NULL;
 	char *tmp_data  = NULL;
 	struct ast_flags opts = { 0, };
 	struct ast_bridge_config bconfig = { { 0, }, };
@@ -1017,22 +1019,20 @@
 		AST_APP_ARG(options);
 	);
 
-	if (ast_strlen_zero(data)) {
-		ast_log(LOG_WARNING, "Bridge require at least 1 argument specifying the other end of the bridge\n");
-		return -1;
+	tmp_data = ast_strdupa(data ?: "");
+	AST_STANDARD_APP_ARGS(args, tmp_data);
+	if (!ast_strlen_zero(args.options)) {
+		ast_app_parse_options(bridge_exec_options, &opts, opt_args, args.options);
 	}
 
-	tmp_data = ast_strdupa(data);
-	AST_STANDARD_APP_ARGS(args, tmp_data);
-	if (!ast_strlen_zero(args.options))
-		ast_app_parse_options(bridge_exec_options, &opts, opt_args, args.options);
-
 	/* make sure we have a valid end point */
-	current_dest_chan = ast_channel_get_by_name_prefix(args.dest_chan,
-		strlen(args.dest_chan));
+	if (!ast_strlen_zero(args.dest_chan)) {
+		current_dest_chan = ast_channel_get_by_name_prefix(args.dest_chan,
+			strlen(args.dest_chan));
+	}
 	if (!current_dest_chan) {
-		ast_log(LOG_WARNING, "Bridge failed because channel %s does not exist\n",
-			args.dest_chan);
+		ast_verb(4, "Bridge failed because channel '%s' does not exist\n",
+			args.dest_chan ?: "");
 		pbx_builtin_setvar_helper(chan, "BRIDGERESULT", "NONEXISTENT");
 		return 0;
 	}

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13513
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I44349a8dc3912397f28852777de04f19e7bb9c73
Gerrit-Change-Number: 13513
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200107/22aec012/attachment.html>