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

[N A]

N A has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/19522 )


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
---
M apps/app_stack.c
1 file changed, 63 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/22/19522/1

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/+/19522
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I1d42a99c9aa9e3708d32718863175158a894e414
Gerrit-Change-Number: 19522
Gerrit-PatchSet: 1
Gerrit-Owner: N A <asterisk at phreaknet.org>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20221102/bffd4707/attachment.html>