[Asterisk-code-review] app_page.c: Simplify dialplan using Page. (asterisk[17])

[Friendly Automation]

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

Change subject: app_page.c: Simplify dialplan using Page.
......................................................................

app_page.c: Simplify dialplan using Page.

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 Page.  Simplify dialplan by making Page
handle these conditions gracefully.

* Made tolerate empty positions in the paged device list.

* Reduced some warnings associated with the 's' option to verbose
messages.  The warning level for those messages really serves no purpose
as that is why the 's' option exists.

ASTERISK-28638

Change-Id: I95b64a6d6800cd1a25279c88889314ae60fc21e3
---
M apps/app_page.c
A doc/CHANGES-staging/app_page_empty_page_list.txt
2 files changed, 23 insertions(+), 14 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_page.c b/apps/app_page.c
index 12ba01d..df601e7 100644
--- a/apps/app_page.c
+++ b/apps/app_page.c
@@ -48,7 +48,7 @@
 			Page series of phones
 		</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>
@@ -270,17 +270,12 @@
 		AST_APP_ARG(timeout);
 	);
 
-	if (ast_strlen_zero(data)) {
-		ast_log(LOG_WARNING, "This application requires at least one argument (destination(s) to page)\n");
-		return -1;
-	}
-
 	if (!(app = pbx_findapp("ConfBridge"))) {
 		ast_log(LOG_WARNING, "There is no ConfBridge application available!\n");
 		return -1;
 	};
 
-	parse = ast_strdupa(data);
+	parse = ast_strdupa(data ?: "");
 
 	AST_STANDARD_APP_ARGS(args, parse);
 
@@ -301,7 +296,7 @@
 
 	/* Count number of extensions in list by number of ampersands + 1 */
 	num_dials = 1;
-	tmp = args.devices;
+	tmp = args.devices ?: "";
 	while (*tmp) {
 		if (*tmp == '&') {
 			num_dials++;
@@ -334,13 +329,20 @@
 		int state = 0;
 		struct ast_dial *dial = NULL;
 
-		/* don't call the originating device */
-		if (!strcasecmp(tech, originator))
+		tech = ast_strip(tech);
+		if (ast_strlen_zero(tech)) {
+			/* No tech/resource in this position. */
 			continue;
+		}
+
+		/* don't call the originating device */
+		if (!strcasecmp(tech, originator)) {
+			continue;
+		}
 
 		/* If no resource is available, continue on */
 		if (!(resource = strchr(tech, '/'))) {
-			ast_log(LOG_WARNING, "Incomplete destination '%s' supplied.\n", tech);
+			ast_log(LOG_WARNING, "Incomplete destination: '%s' supplied.\n", tech);
 			continue;
 		}
 
@@ -348,9 +350,11 @@
 		if (ast_test_flag(&options.flags, PAGE_SKIP)) {
 			state = ast_device_state(tech);
 			if (state == AST_DEVICE_UNKNOWN) {
-				ast_log(LOG_WARNING, "Destination '%s' has device state '%s'. Paging anyway.\n", tech, ast_devstate2str(state));
+				ast_verb(3, "Destination '%s' has device state '%s'. Paging anyway.\n",
+					tech, ast_devstate2str(state));
 			} else if (state != AST_DEVICE_NOT_INUSE) {
-				ast_log(LOG_WARNING, "Destination '%s' has device state '%s'.\n", tech, ast_devstate2str(state));
+				ast_verb(3, "Destination '%s' has device state '%s'.\n",
+					tech, ast_devstate2str(state));
 				continue;
 			}
 		}
@@ -365,7 +369,7 @@
 
 		/* Append technology and resource */
 		if (ast_dial_append(dial, tech, resource, NULL) == -1) {
-			ast_log(LOG_ERROR, "Failed to add %s to outbound dial\n", tech);
+			ast_log(LOG_ERROR, "Failed to add %s/%s to outbound dial\n", tech, resource);
 			ast_dial_destroy(dial);
 			continue;
 		}
diff --git a/doc/CHANGES-staging/app_page_empty_page_list.txt b/doc/CHANGES-staging/app_page_empty_page_list.txt
new file mode 100644
index 0000000..73e8420
--- /dev/null
+++ b/doc/CHANGES-staging/app_page_empty_page_list.txt
@@ -0,0 +1,5 @@
+Subject: app_page
+
+The Page 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.

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

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: I95b64a6d6800cd1a25279c88889314ae60fc21e3
Gerrit-Change-Number: 13558
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/cfcb947b/attachment-0001.html>