[Home]

Summary:ASTERISK-05010: [patch] app_while first loop executes even if evaluates to "0"
Reporter:John Todd (jtodd)Labels:
Date Opened:2005-09-06 16:11:33Date Closed:2008-01-15 15:48:09.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Applications/app_while
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 20050907__endwhile.diff.txt
( 1) 20050908__endwhile__2.diff.txt
( 2) 20050908__endwhile.diff.txt
Description:I seem to be having a conceptual problem with the "While" and "EndWhile" applications.  It seems that on the first cycle, even if the result of the "While" is false that the enclosed applications will get run.  Is this expected?  It seems to be counter-intuitive, but I don't know what the intent of the While routines is.  I could of course put a "GotoIf" before the While loop to check to ensure that the first expression is true before entry into the While loop, but that seems redundant and ugly since the while point of While and EndWhile is to avoid the inelegance of GotoIf, I thought.

If anyone can't come up with a better explanation, I'll open a ticket on this but I'd like to first make sure that this behavior is not expected.


exten => 2231,1,Set(staticnumber=0)
exten => 2231,n,Set(counter=1)
exten => 2231,n,While($["${counter}"<"${staticnumber}"])
exten => 2231,n,NoOp("This part of the code should never run!")
exten => 2231,n,Set(counter=$[${counter}+1])
exten => 2231,n,EndWhile
exten => 2231,n,NoOp("This part of the code should be the only thing that gets run!")


Console output from dialing 2231:

   -- Executing Set("SIP/2203-c134", "staticnumber=0") in new stack
   -- Executing Set("SIP/2203-c134", "counter=1") in new stack
   -- Executing While("SIP/2203-c134", "0") in new stack
   -- Executing NoOp("SIP/2203-c134", ""This part of the code should never run!"") in new stack
   -- Executing Set("SIP/2203-c134", "counter=2") in new stack
   -- Executing EndWhile("SIP/2203-c134", "") in new stack
   -- Executing NoOp("SIP/2203-c134", ""This part of the code should be the only thing that gets run!"") in new stack
*CLI> show version
Asterisk CVS HEAD built by root@some.host.com on a i686 running Linux on 2005-09-03 23:27:34 UTC
Comments:By: Clod Patry (junky) 2005-09-06 21:01:06

The problem here, is the first loop, it doesn't know that the EndWhile is at priority 6 to directly go there. It's only its first loop it knows it.
By: Michael Jerris (mikej) 2005-09-06 21:30:47

we could just change while and end while to do and until... but I am guessing that is not what you are looking for right?
By: Clod Patry (junky) 2005-09-06 21:55:53

no, its not. i understand what he wants.
while(0);
print "foo";
}
print "bar";

we should just get bar, not foobar, he probably wants that concept too.

Maybe we could analyse on what priority the endwhile is on, and goto that priority if (!ast_true(condition))
but that would require that all while is called with an endwhile, which makes sense to me.
By: John Todd (jtodd) 2005-09-07 16:01:47

Junky: you have summarized the problem more clearly than I have with that example.

I would agree that all While applications should have an EndWhile following, otherwise a warning (or error?) should be produced.  This would allow evaluation on the first cycle.  If there is no mandatory EndWhile, how would While ever be used in the first place?
By: Anthony Minessale (anthm) 2005-09-07 19:02:42

Here is the only way I know you can fix it:

Since you have never hit the endwhile you have no way to tell where it is so you can help it out by setting the end pointer manually

The nested counter starts with 0 so for an ordinary while it would be END_WHILE_0 if it was nested it would be END_WHILE_1 etc.

so Set(END_WHILE_0=6) somewhere in the exten would tell the app that the end pointer for the top level while loop is at priority 6

you can also use +

Set(END_WHILE_0=+4) to jump 4 priorities past the top level while if the condition is false.

This is a limitation of the way the dialplan works it's all I can come up with.
By: Michael Jerris (mikej) 2005-09-07 22:22:56

Reading the app description, the behavior is consistant with the intention and documentation.  Maybe Do-->Until is a better name for this (but we would need to work the condition into the until line to make it work like that).  Changing this to a feature request as it does work as documented:

"Start a While Loop.  Execution will return to this point when\n"
"EndWhile is called until expr is no longer true.\n";
By: Tilghman Lesher (tilghman) 2005-09-07 23:24:33

Here's a start of a patch to find the right matching EndWhile.  It's far from perfect:  it only works when the priorities are sequential from the beginning to the end and it doesn't match on callerid.  However, it may work for what you're trying to do.  For this to go much further is quite a bit of work and will require quite a bit of semantic interpretation -- very difficult code to write.

I have not tested this patch, but you're free to try it out.  At the very least, it demonstrates that we could find the matching EndWhile in certain limited circumstances without too much work.
By: John Todd (jtodd) 2005-09-08 10:49:43

OK, it sort-of works- it skips, but doesn't continue.  The dialplan example I provided above now produces this console output:

   -- Executing Set("SIP/2203-f95e", "staticnumber=0") in new stack
   -- Executing Set("SIP/2203-f95e", "counter=1") in new stack
   -- Executing While("SIP/2203-f95e", "0") in new stack

...but then locks up.  Hanging up the inbound SIP channel does not cause a Hangup in the dialplan, and I have to do a "soft hangup" to clear the channel.   I would expect to see the output of "exten => 2231,n,NoOp("This part of the code should be the only thing that gets run!")" or of the EndWhile priority.

I believe that some sort of fix (probably along your lines, Tilghman) is required to "While/Endwhile" to make it behave in a way that is expected by a user.  The whole concept of "While" in Asterisk is a bit complex, and I was initially very pleased to see it since it saved me quite a bit of work in my dialplans.  However, it seems that complexity is now passed into the underlying parsing code instead of remaining in the dialplan.
By: Tilghman Lesher (tilghman) 2005-09-08 11:43:53

Ah.  Yeah.  Infinite loop.  Fixed.
By: Tilghman Lesher (tilghman) 2005-09-08 11:52:43

Third patch is with callerid matching, so it's closer.  Still, most of the caveats apply.
By: John Todd (jtodd) 2005-09-08 21:09:14

OK, this third patch behaves more like I'd expect:

ms1*CLI>
   -- Executing Set("SIP/2203-20ba", "staticnumber=0") in new stack
   -- Executing Set("SIP/2203-20ba", "counter=1") in new stack
   -- Executing While("SIP/2203-20ba", "0") in new stack
   -- Jumping to priority 6
   -- Executing NoOp("SIP/2203-20ba", ""This part of the code should be the only thing that gets run!"") in new stack
ms1*CLI>

With this patch, there perhaps are some fundamental questions:

1) should this patch be applied in it's current state, which may not encompass all cases (Macros, jumps out of the "While" loop with Goto, etc.)

2) does a significant amount of additional work need to be given to the patch before it is implemented, which catches "all" cases of jumps outside of a While/Endwhile loop?

3) Does While/Endwhile even have a place in Asterisk due to it's complex nature?  Two identical calls to "GotoIf" can replace While and Endwhile from the quick thinking I've done on it.

I'd vote for #1, but I perhaps will have a better understanding of the shortcomings than someone who is coming at the issue without any background on it, which could lead to confounding debugging of dialplan path progress.
By: Michael Jerris (mikej) 2005-09-08 21:25:46

goto's in a while loop with a false condition to start should not effect the flow, as they are never executed... the same with any other dialpalan flow modifiers (macro calls, ect) witin the while loop that is not being executed.  If it is false to start with, it should skip over it.
By: Tilghman Lesher (tilghman) 2005-09-08 21:31:29

MikeJ is right.  Forget my caveats; I was over-analysing the code.  The code is already where it needs to be.
By: Kevin P. Fleming (kpfleming) 2005-09-13 22:26:19

Committed to CVS HEAD, thanks!
By: Digium Subversion (svnbot) 2008-01-15 15:48:09.000-0600

Repository: asterisk
Revision: 6586

U   trunk/apps/app_while.c

------------------------------------------------------------------------
r6586 | kpfleming | 2008-01-15 15:48:08 -0600 (Tue, 15 Jan 2008) | 2 lines

make While() able to find the matching EndWhile() for when the condition is zero (issue ASTERISK-5010)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=6586