[Asterisk-code-review] app_chanisavail.c: Simplify dialplan using ChanIsAvail. (asterisk[master])
George Joseph
asteriskteam at digium.com
Tue Jan 7 14:28:59 CST 2020
George Joseph has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13565 )
Change subject: app_chanisavail.c: Simplify dialplan using ChanIsAvail.
......................................................................
app_chanisavail.c: Simplify dialplan using ChanIsAvail.
Dialplan has to be careful about passing an empty device list or empty
positions in the list. As a result, dialplan has to check for these
conditions before using ChanIsAvail. Simplify dialplan by making
ChanIsAvail handle these conditions gracefully.
* Made tolerate empty positions in the device list.
* Simplified the code and eliminated some unnecessary indention.
ASTERISK-28638
Change-Id: I9e4b67e2cbf26b2417c2d03485b8568e898931d3
---
M apps/app_chanisavail.c
A doc/CHANGES-staging/app_chanisavail_empty_device_list.txt
2 files changed, 79 insertions(+), 69 deletions(-)
Approvals:
Sean Bright: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved; Approved for Submit
diff --git a/apps/app_chanisavail.c b/apps/app_chanisavail.c
index 1c7c1ec..c75bb79 100644
--- a/apps/app_chanisavail.c
+++ b/apps/app_chanisavail.c
@@ -51,16 +51,18 @@
Check channel availability
</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 check. These must be in the format of
+ <literal>Technology/Resource</literal>, where <replaceable>Technology</replaceable>
+ represents a particular channel driver, and <replaceable>Resource</replaceable>
+ represents a resource available to that particular channel driver.</para>
+ </argument>
<argument name="Technology2/Resource2" multiple="true">
<para>Optional extra devices to check</para>
<para>If you need more than one enter them as
Technology2/Resource2&Technology3/Resource3&.....</para>
</argument>
- <para>Specification of the device(s) to check. These must be in the format of
- <literal>Technology/Resource</literal>, where <replaceable>Technology</replaceable>
- represents a particular channel driver, and <replaceable>Resource</replaceable>
- represents a resource available to that particular channel driver.</para>
</parameter>
<parameter name="options" required="false">
<optionlist>
@@ -99,25 +101,28 @@
static int chanavail_exec(struct ast_channel *chan, const char *data)
{
- int inuse=-1, option_state=0, string_compare=0, option_all_avail=0;
+ int inuse = -1;
+ int option_state = 0;
+ int string_compare = 0;
+ int option_all_avail = 0;
int status;
- char *info, tmp[512], trychan[512], *peers, *tech, *number, *rest, *cur;
+ char *info;
+ char trychan[512];
+ char *rest;
+ char *tech;
+ char *number;
struct ast_str *tmp_availchan = ast_str_alloca(2048);
struct ast_str *tmp_availorig = ast_str_alloca(2048);
struct ast_str *tmp_availstat = ast_str_alloca(2048);
struct ast_str *tmp_availcause = ast_str_alloca(2048);
struct ast_channel *tempchan;
+ struct ast_custom_function *cdr_prop_func = ast_custom_function_find("CDR_PROP");
AST_DECLARE_APP_ARGS(args,
AST_APP_ARG(reqchans);
AST_APP_ARG(options);
);
- if (ast_strlen_zero(data)) {
- ast_log(LOG_WARNING, "ChanIsAvail requires an argument (DAHDI/1&DAHDI/2)\n");
- return -1;
- }
-
- info = ast_strdupa(data);
+ info = ast_strdupa(data ?: "");
AST_STANDARD_APP_ARGS(args, info);
@@ -132,68 +137,68 @@
string_compare = 1;
}
}
- peers = args.reqchans;
- if (peers) {
- struct ast_custom_function *cdr_prop_func = ast_custom_function_find("CDR_PROP");
- cur = peers;
- do {
- /* remember where to start next time */
- rest = strchr(cur, '&');
- if (rest) {
- *rest = 0;
- rest++;
+ rest = args.reqchans;
+ if (!rest) {
+ rest = "";
+ }
+ while ((tech = strsep(&rest, "&"))) {
+ tech = ast_strip(tech);
+
+ number = strchr(tech, '/');
+ if (!number) {
+ if (!ast_strlen_zero(tech)) {
+ ast_log(LOG_WARNING, "Invalid ChanIsAvail technology/resource argument: '%s'\n",
+ tech);
}
- tech = cur;
- number = strchr(tech, '/');
- if (!number) {
- ast_log(LOG_WARNING, "ChanIsAvail argument takes format ([technology]/[device])\n");
- return -1;
+
+ ast_str_append(&tmp_availstat, 0, "%s%d",
+ ast_str_strlen(tmp_availstat) ? "&" : "", AST_DEVICE_INVALID);
+ continue;
+ }
+ *number++ = '\0';
+
+ status = AST_DEVICE_UNKNOWN;
+
+ if (string_compare) {
+ /* ast_parse_device_state checks for "SIP/1234" as a channel name.
+ ast_device_state will ask the SIP driver for the channel state. */
+
+ snprintf(trychan, sizeof(trychan), "%s/%s", tech, number);
+ status = inuse = ast_parse_device_state(trychan);
+ } else if (option_state) {
+ /* If the pbx says in use then don't bother trying further.
+ This is to permit testing if someone's on a call, even if the
+ channel can permit more calls (ie callwaiting, sip calls, etc). */
+
+ snprintf(trychan, sizeof(trychan), "%s/%s", tech, number);
+ status = inuse = ast_device_state(trychan);
+ }
+ ast_str_append(&tmp_availstat, 0, "%s%d",
+ ast_str_strlen(tmp_availstat) ? "&" : "", status);
+ if ((inuse <= (int) AST_DEVICE_NOT_INUSE)
+ && (tempchan = ast_request(tech, ast_channel_nativeformats(chan), NULL, chan, number, &status))) {
+ ast_str_append(&tmp_availchan, 0, "%s%s",
+ ast_str_strlen(tmp_availchan) ? "&" : "", ast_channel_name(tempchan));
+
+ ast_str_append(&tmp_availorig, 0, "%s%s/%s",
+ ast_str_strlen(tmp_availorig) ? "&" : "", tech, number);
+
+ ast_str_append(&tmp_availcause, 0, "%s%d",
+ ast_str_strlen(tmp_availcause) ? "&" : "", status);
+
+ /* Disable CDR for this temporary channel. */
+ if (cdr_prop_func) {
+ ast_func_write(tempchan, "CDR_PROP(disable)", "1");
}
- *number = '\0';
- number++;
- status = AST_DEVICE_UNKNOWN;
+ ast_hangup(tempchan);
+ tempchan = NULL;
- if (string_compare) {
- /* ast_parse_device_state checks for "SIP/1234" as a channel name.
- ast_device_state will ask the SIP driver for the channel state. */
-
- snprintf(trychan, sizeof(trychan), "%s/%s",cur,number);
- status = inuse = ast_parse_device_state(trychan);
- } else if (option_state) {
- /* If the pbx says in use then don't bother trying further.
- This is to permit testing if someone's on a call, even if the
- channel can permit more calls (ie callwaiting, sip calls, etc). */
-
- snprintf(trychan, sizeof(trychan), "%s/%s",cur,number);
- status = inuse = ast_device_state(trychan);
+ if (!option_all_avail) {
+ break;
}
- snprintf(tmp, sizeof(tmp), "%d", status);
- ast_str_append(&tmp_availstat, 0, "%s%s", ast_str_strlen(tmp_availstat) ? "&" : "", tmp);
- if ((inuse <= 1) && (tempchan = ast_request(tech, ast_channel_nativeformats(chan), NULL, chan, number, &status))) {
- ast_str_append(&tmp_availchan, 0, "%s%s", ast_str_strlen(tmp_availchan) ? "&" : "", ast_channel_name(tempchan));
-
- snprintf(tmp, sizeof(tmp), "%s/%s", tech, number);
- ast_str_append(&tmp_availorig, 0, "%s%s", ast_str_strlen(tmp_availorig) ? "&" : "", tmp);
-
- snprintf(tmp, sizeof(tmp), "%d", status);
- ast_str_append(&tmp_availcause, 0, "%s%s", ast_str_strlen(tmp_availcause) ? "&" : "", tmp);
-
- /* Disable CDR for this temporary channel. */
- if (cdr_prop_func) {
- ast_func_write(tempchan, "CDR_PROP(disable)", "1");
- }
-
- ast_hangup(tempchan);
- tempchan = NULL;
-
- if (!option_all_avail) {
- break;
- }
- }
- cur = rest;
- } while (cur);
+ }
}
pbx_builtin_setvar_helper(chan, "AVAILCHAN", ast_str_buffer(tmp_availchan));
diff --git a/doc/CHANGES-staging/app_chanisavail_empty_device_list.txt b/doc/CHANGES-staging/app_chanisavail_empty_device_list.txt
new file mode 100644
index 0000000..bf06c31
--- /dev/null
+++ b/doc/CHANGES-staging/app_chanisavail_empty_device_list.txt
@@ -0,0 +1,5 @@
+Subject: app_chanisavail
+
+The ChanIsAvail application now tolerates empty positions in the supplied
+device list. Dialplan can now be simplified by not having to check for
+empty positions in the device list.
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13565
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I9e4b67e2cbf26b2417c2d03485b8568e898931d3
Gerrit-Change-Number: 13565
Gerrit-PatchSet: 2
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.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/49771448/attachment-0001.html>
More information about the asterisk-code-review
mailing list