[Asterisk-code-review] app_stack: Print proper exit location for PBXless channels. (asterisk[18])

[Friendly Automation]

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

Change subject: app_stack: Print proper exit location for PBXless channels.
......................................................................

app_stack: Print proper exit location for PBXless channels.

When gosub is executed on channels without a PBX, the context,
extension, and priority are initialized to the channel driver's
default location for that endpoint. As a result, the last Return
will restore this location and the Gosub logs will print out bogus
information about our exit point.

To fix this, on channels that don't have a PBX, the execution
location is left intact on the last return if there are no
further stack frames left. This allows the correct location
to be printed out to the user, rather than the bogus default
context.

ASTERISK-30076 #close

Change-Id: I1d42a99c9aa9e3708d32718863175158a894e414
(cherry picked from commit 12d18b0a40381eb90145e352bf144a71f8dd2555)
---
M apps/app_stack.c
1 file changed, 64 insertions(+), 8 deletions(-)

Approvals:
  Joshua Colp: 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_stack.c b/apps/app_stack.c
index a84a45c..70aff6f 100644
--- a/apps/app_stack.c
+++ b/apps/app_stack.c
@@ -377,6 +377,26 @@
 	return res;
 }
 
+static int frames_left(struct ast_channel *chan)
+{
+	struct ast_datastore *stack_store;
+	struct gosub_stack_list *oldlist;
+	int exists;
+
+	ast_channel_lock(chan);
+	stack_store = ast_channel_datastore_find(chan, &stack_info, NULL);
+	if (!stack_store) {
+		ast_channel_unlock(chan);
+		return -1;
+	}
+	oldlist = stack_store->data;
+	AST_LIST_LOCK(oldlist);
+	exists = oldlist->first ? 1 : 0;
+	AST_LIST_UNLOCK(oldlist);
+	ast_channel_unlock(chan);
+	return exists;
+}
+
 static int return_exec(struct ast_channel *chan, const char *data)
 {
 	struct ast_datastore *stack_store;
@@ -384,6 +404,7 @@
 	struct gosub_stack_list *oldlist;
 	const char *retval = data;
 	int res = 0;
+	int lastframe;
 
 	ast_channel_lock(chan);
 	if (!(stack_store = ast_channel_datastore_find(chan, &stack_info, NULL))) {
@@ -395,6 +416,7 @@
 	oldlist = stack_store->data;
 	AST_LIST_LOCK(oldlist);
 	oldframe = AST_LIST_REMOVE_HEAD(oldlist, entries);
+	lastframe = oldlist->first ? 0 : 1;
 	AST_LIST_UNLOCK(oldlist);
 
 	if (!oldframe) {
@@ -412,12 +434,19 @@
 	 * what was there before.  Channels that do not have a PBX may
 	 * not have the context or exten set.
 	 */
-	ast_channel_context_set(chan, oldframe->context);
-	ast_channel_exten_set(chan, oldframe->extension);
-	if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP)) {
-		--oldframe->priority;
+	if (ast_channel_pbx(chan) || !lastframe) {
+		/* If there's no PBX, the "old location" is simply
+		 * the configured context for the device, such as
+		 * for pre-dial handlers, and restoring this location
+		 * is nonsensical. So if no PBX and there are no further
+		 * frames, leave the location as it is. */
+		ast_channel_context_set(chan, oldframe->context);
+		ast_channel_exten_set(chan, oldframe->extension);
+		if (ast_test_flag(ast_channel_flags(chan), AST_FLAG_IN_AUTOLOOP)) {
+			--oldframe->priority;
+		}
+		ast_channel_priority_set(chan, oldframe->priority);
 	}
-	ast_channel_priority_set(chan, oldframe->priority);
 	ast_set2_flag(ast_channel_flags(chan), oldframe->in_subroutine, AST_FLAG_SUBROUTINE_EXEC);
 
 	gosub_release_frame(chan, oldframe);
@@ -1068,10 +1097,13 @@
 				ast_channel_priority(chan), ast_channel_name(chan));
 		}
 
-		/* Did the routine return? */
-		if (ast_channel_priority(chan) == saved_priority
+		/* Did the routine return?
+		 * For things like predial where there's no PBX on the channel yet,
+		 * the last return leaves the location alone so we can print it out correctly here.
+		 * So to ensure we finished properly, make sure there are no frames left in that case. */
+		if ((!ast_channel_pbx(chan) && !frames_left(chan)) || (ast_channel_priority(chan) == saved_priority
 			&& !strcmp(ast_channel_context(chan), saved_context)
-			&& !strcmp(ast_channel_exten(chan), saved_exten)) {
+			&& !strcmp(ast_channel_exten(chan), saved_exten))) {
 			ast_verb(3, "%s Internal %s(%s) complete GOSUB_RETVAL=%s\n",
 				ast_channel_name(chan), app_gosub, sub_args,
 				S_OR(pbx_builtin_getvar_helper(chan, "GOSUB_RETVAL"), ""));

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

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: I1d42a99c9aa9e3708d32718863175158a894e414
Gerrit-Change-Number: 19531
Gerrit-PatchSet: 2
Gerrit-Owner: N A <asterisk at phreaknet.org>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221102/c098d8d0/attachment.html>