[Asterisk-code-review] app_dial.c: Simplify dialplan using Dial. (asterisk[13])

Friendly Automation asteriskteam at digium.com
Tue Jan 7 11:36:03 CST 2020


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

Change subject: app_dial.c: Simplify dialplan using Dial.
......................................................................

app_dial.c: Simplify dialplan using Dial.

Dialplan has to be careful about passing an empty destination list or
empty positions in the list.  As a result, dialplan has to check for
these conditions before using Dial.  Simplify dialplan by making Dial
handle these conditions gracefully.

* Made tolerate empty positions in the dialed device list.

* Reduced some message log levels from notice to verbose.

ASTERISK-28638

Change-Id: I6edc731aff451f8bdfaee5498078dd18c3a11ab9
---
M apps/app_dial.c
A doc/CHANGES-staging/app_dial_empty_dial_list.txt
2 files changed, 36 insertions(+), 21 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/apps/app_dial.c b/apps/app_dial.c
index 77b3ace..d85bca1 100644
--- a/apps/app_dial.c
+++ b/apps/app_dial.c
@@ -75,7 +75,7 @@
 			Attempt to connect to another device or endpoint and bridge the call.
 		</synopsis>
 		<syntax>
-			<parameter name="Technology/Resource" required="true" argsep="&">
+			<parameter name="Technology/Resource" required="false" argsep="&">
 				<argument name="Technology/Resource" required="true">
 					<para>Specification of the device(s) to dial.  These must be in the format of
 					<literal>Technology/Resource</literal>, where <replaceable>Technology</replaceable>
@@ -1000,7 +1000,8 @@
 			 * any Dial operations that happen later won't record CC interfaces.
 			 */
 			ast_ignore_cc(o->chan);
-			ast_log(LOG_NOTICE, "Not accepting call completion offers from call-forward recipient %s\n", ast_channel_name(o->chan));
+			ast_verb(3, "Not accepting call completion offers from call-forward recipient %s\n",
+				ast_channel_name(o->chan));
 		} else
 			ast_log(LOG_NOTICE,
 				"Forwarding failed to create channel to dial '%s/%s' (cause = %d)\n",
@@ -2016,7 +2017,7 @@
 		/* well, there seems basically two choices. Just patch the caller thru immediately,
 			  or,... put 'em thru to voicemail. */
 		/* since the callee may have hung up, let's do the voicemail thing, no database decision */
-		ast_log(LOG_NOTICE, "privacy: no valid response from the callee. Sending the caller to voicemail, the callee isn't responding\n");
+		ast_verb(3, "privacy: no valid response from the callee. Sending the caller to voicemail, the callee isn't responding\n");
 		/* XXX should we set status to DENY ? */
 		/* XXX what about the privacy flags ? */
 		break;
@@ -2307,12 +2308,6 @@
 		return -1;
 	}
 
-	if (ast_strlen_zero(data)) {
-		ast_log(LOG_WARNING, "Dial requires an argument (technology/resource)\n");
-		pbx_builtin_setvar_helper(chan, "DIALSTATUS", pa.status);
-		return -1;
-	}
-
 	if (ast_check_hangup_locked(chan)) {
 		/*
 		 * Caller hung up before we could dial.  If dial is executed
@@ -2331,7 +2326,7 @@
 		return -1;
 	}
 
-	parse = ast_strdupa(data);
+	parse = ast_strdupa(data ?: "");
 
 	AST_STANDARD_APP_ARGS(args, parse);
 
@@ -2341,12 +2336,6 @@
 		goto done;
 	}
 
-	if (ast_strlen_zero(args.peers)) {
-		ast_log(LOG_WARNING, "Dial requires an argument (technology/resource)\n");
-		pbx_builtin_setvar_helper(chan, "DIALSTATUS", pa.status);
-		goto done;
-	}
-
 	if (ast_cc_call_init(chan, &ignore_cc)) {
 		goto done;
 	}
@@ -2372,7 +2361,7 @@
 	if (ast_test_flag64(&opts, OPT_DURATION_STOP) && !ast_strlen_zero(opt_args[OPT_ARG_DURATION_STOP])) {
 		calldurationlimit.tv_sec = atoi(opt_args[OPT_ARG_DURATION_STOP]);
 		if (!calldurationlimit.tv_sec) {
-			ast_log(LOG_WARNING, "Dial does not accept S(%s), hanging up.\n", opt_args[OPT_ARG_DURATION_STOP]);
+			ast_log(LOG_WARNING, "Dial does not accept S(%s)\n", opt_args[OPT_ARG_DURATION_STOP]);
 			pbx_builtin_setvar_helper(chan, "DIALSTATUS", pa.status);
 			goto done;
 		}
@@ -2519,15 +2508,24 @@
 
 	/* loop through the list of dial destinations */
 	rest = args.peers;
-	while ((cur = strsep(&rest, "&")) ) {
+	while ((cur = strsep(&rest, "&"))) {
 		struct ast_channel *tc; /* channel for this destination */
-		/* Get a technology/resource pair */
-		char *number = cur;
-		char *tech = strsep(&number, "/");
+		char *number;
+		char *tech;
 		size_t tech_len;
 		size_t number_len;
 		struct ast_format_cap *nativeformats;
 
+		cur = ast_strip(cur);
+		if (ast_strlen_zero(cur)) {
+			/* No tech/resource in this position. */
+			continue;
+		}
+
+		/* Get a technology/resource pair */
+		number = cur;
+		tech = strsep(&number, "/");
+
 		num_dialed++;
 		if (ast_strlen_zero(number)) {
 			ast_log(LOG_WARNING, "Dial argument takes format (technology/resource)\n");
@@ -2726,6 +2724,17 @@
 		AST_LIST_INSERT_TAIL(&out_chans, tmp, node);
 	}
 
+	if (AST_LIST_EMPTY(&out_chans)) {
+		ast_verb(3, "No devices or endpoints to dial (technology/resource)\n");
+		if (continue_exec) {
+			/* There is no point in having RetryDial try again */
+			*continue_exec = 1;
+		}
+		strcpy(pa.status, "CHANUNAVAIL");
+		res = 0;
+		goto out;
+	}
+
 	/*
 	 * PREDIAL: Run gosub on all of the callee channels
 	 *
diff --git a/doc/CHANGES-staging/app_dial_empty_dial_list.txt b/doc/CHANGES-staging/app_dial_empty_dial_list.txt
new file mode 100644
index 0000000..dc68ee6
--- /dev/null
+++ b/doc/CHANGES-staging/app_dial_empty_dial_list.txt
@@ -0,0 +1,6 @@
+Subject: app_dial
+
+The Dial application now tolerates empty positions in the supplied
+destination list.  Dialplan can now be simplified by not having to check
+for empty positions in the destination list.  If there are no endpoints to
+dial then DIALSTATUS is set to CHANUNAVAIL.

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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I6edc731aff451f8bdfaee5498078dd18c3a11ab9
Gerrit-Change-Number: 13560
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/c170b4f0/attachment.html>


More information about the asterisk-code-review mailing list