[Asterisk-code-review] app originate: Set ORIGINATE STATUS correctly on failure (asterisk[15])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/6731 )

Change subject: app_originate: Set ORIGINATE_STATUS correctly on failure
......................................................................

app_originate: Set ORIGINATE_STATUS correctly on failure

We were ignoring the return value from ast_pbx_outgoing_exten() and
ast_pbx_outgoing_app() which could fail before setting the reason code.
This resulted in failures being reported as success.

ASTERISK-25266 #close
Reported by: Allen Ford

Change-Id: Idf16237b7e41b527d2c69c865829128686beeb3b
---
M apps/app_originate.c
1 file changed, 23 insertions(+), 9 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  Richard Mudgett: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/apps/app_originate.c b/apps/app_originate.c
index 8edccc7..30fa565 100644
--- a/apps/app_originate.c
+++ b/apps/app_originate.c
@@ -155,6 +155,7 @@
 	char *parse;
 	char *chantech, *chandata;
 	int res = -1;
+	int continue_in_dialplan = 0;
 	int outgoing_status = 0;
 	unsigned int timeout = 30;
 	static const char default_exten[] = "s";
@@ -223,6 +224,12 @@
 		predial_callee = opt_args[OPT_ARG_PREDIAL_CALLEE];
 	}
 
+	if (strcasecmp(args.type, "exten") && strcasecmp(args.type, "app")) {
+		ast_log(LOG_ERROR, "Incorrect type, it should be 'exten' or 'app': %s\n",
+				args.type);
+		goto return_cleanup;
+	}
+
 	if (!strcasecmp(args.type, "exten")) {
 		int priority = 1; /* Initialized in case priority not specified */
 		const char *exten = args.arg2;
@@ -241,25 +248,32 @@
 		ast_debug(1, "Originating call to '%s/%s' and connecting them to extension %s,%s,%d\n",
 				chantech, chandata, args.arg1, exten, priority);
 
-		ast_pbx_outgoing_exten_predial(chantech, cap_slin, chandata,
+		res = ast_pbx_outgoing_exten_predial(chantech, cap_slin, chandata,
 				timeout * 1000, args.arg1, exten, priority, &outgoing_status,
 				AST_OUTGOING_WAIT, NULL, NULL, NULL, NULL, NULL, 0, NULL,
 				predial_callee);
-	} else if (!strcasecmp(args.type, "app")) {
+	} else {
 		ast_debug(1, "Originating call to '%s/%s' and connecting them to %s(%s)\n",
 				chantech, chandata, args.arg1, S_OR(args.arg2, ""));
 
-		ast_pbx_outgoing_app_predial(chantech, cap_slin, chandata,
+		res = ast_pbx_outgoing_app_predial(chantech, cap_slin, chandata,
 				timeout * 1000, args.arg1, args.arg2, &outgoing_status,
 				AST_OUTGOING_WAIT, NULL, NULL, NULL, NULL, NULL, NULL,
 				predial_callee);
-	} else {
-		ast_log(LOG_ERROR, "Incorrect type, it should be 'exten' or 'app': %s\n",
-				args.type);
-		goto return_cleanup;
 	}
 
-	res = 0;
+	/*
+	 * Getting here means that we have passed the various validation checks and
+	 * have at least attempted the dial. If we have a reason (outgoing_status),
+	 * we clear our error indicator so that we ultimately report the right thing
+	 * to the caller.
+	 */
+	if (res && outgoing_status) {
+		res = 0;
+	}
+
+	/* We need to exit cleanly if we've gotten this far */
+	continue_in_dialplan = 1;
 
 return_cleanup:
 	if (res) {
@@ -292,7 +306,7 @@
 	ao2_cleanup(cap_slin);
 	ast_autoservice_stop(chan);
 
-	return res;
+	return continue_in_dialplan ? 0 : -1;
 }
 
 static int unload_module(void)

-- 
To view, visit https://gerrit.asterisk.org/6731
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: Idf16237b7e41b527d2c69c865829128686beeb3b
Gerrit-Change-Number: 6731
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171011/9412c0c2/attachment.html>