[falk@debian.org: Bug#107528: "while do done" gets into uninterruptable loop]

[falk@debian.org: Bug#107528: "while do done" gets into uninterruptable loop]

[Clint Adams]

----- Forwarded message from Falk Hueffner <falk@debian.org> -----

Package: zsh
Version: 4.0.2-2
Severity: normal

When I type "while do done", zsh loops without reacting to signals
like Ctrl-c, Ctrl-\, kill -TERM. No problem with "while :; do
done". The man page says

       while list do list done

and list might be empty, so this should be valid syntax.

I just noticed it returns immediately if $? happens to be non-zero. I
think it should always loop infinitely. In any case, it should be
interruptable.

	Falk


-- System Information
Debian Release: testing/unstable
Architecture: alpha
Kernel: Linux borkum 2.4.7 #2 Sat Jul 21 14:28:04 CEST 2001 alpha
Locale: LANG=C, LC_CTYPE=C

Re: [falk@debian.org: Bug#107528: "while do done" gets into uninterruptable loop]

[Sven Wischnowsky]

Clint Adams wrote:

> ...
> 
> When I type "while do done", zsh loops without reacting to signals
> like Ctrl-c, Ctrl-\, kill -TERM. No problem with "while :; do
> done". The man page says
> 
>        while list do list done
> 
> and list might be empty, so this should be valid syntax.
> 
> I just noticed it returns immediately if $? happens to be non-zero. I
> think it should always loop infinitely. In any case, it should be
> interruptable.

Hmhm.  This is because in such a loop none of the flags that are tested
in zhandler() ever gets set (line 519).

The patch below looks stupid (and it may well be), but it's the easiest
I can think of.  I think I better not commit this until I get replies.
(Most of it is re-indentation of that loop, there's just a tiny bit of
added code at the top.)

Another thing we could try is to just add `|| loops' to that line in
signals.c but I'm not sure about what unwanted consequences that would
have.


Bye
  Sven

Index: Src/loop.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/loop.c,v
retrieving revision 1.5
diff -u -r1.5 loop.c
--- Src/loop.c	2001/06/25 16:07:52	1.5
+++ Src/loop.c	2001/08/07 13:48:28
@@ -380,37 +380,53 @@
     cmdpush(isuntil ? CS_UNTIL : CS_WHILE);
     loops++;
     loop = state->pc;
-    for (;;) {
-	state->pc = loop;
-	noerrexit = 1;
-	execlist(state, 1, 0);
-	noerrexit = olderrexit;
-	if (!((lastval == 0) ^ isuntil)) {
-	    if (breaks)
-		breaks--;
-	    lastval = oldval;
-	    break;
-	}
-	if (retflag) {
-	    lastval = oldval;
-	    break;
-	}
-	execlist(state, 1, 0);
-	if (breaks) {
-	    breaks--;
-	    if (breaks || !contflag)
-		break;
-	    contflag = 0;
-	}
-	if (errflag) {
-	    lastval = 1;
-	    break;
-	}
-	if (retflag)
-	    break;
-	freeheap();
-	oldval = lastval;
-    }
+
+    if (loop[0] == WC_END && loop[1] == WC_END) {
+
+        /* This is an empty loop.  Make sure the signal handler sets the
+        * flags and then just wait for someone hitting ^C. */
+
+        int old_simple_pline = simple_pline;
+
+        simple_pline = 1;
+
+        while (!breaks)
+            ;
+        breaks--;
+
+        simple_pline = old_simple_pline;
+    } else
+        for (;;) {
+            state->pc = loop;
+            noerrexit = 1;
+            execlist(state, 1, 0);
+            noerrexit = olderrexit;
+            if (!((lastval == 0) ^ isuntil)) {
+                if (breaks)
+                    breaks--;
+                lastval = oldval;
+                break;
+            }
+            if (retflag) {
+                lastval = oldval;
+                break;
+            }
+            execlist(state, 1, 0);
+            if (breaks) {
+                breaks--;
+                if (breaks || !contflag)
+                    break;
+                contflag = 0;
+            }
+            if (errflag) {
+                lastval = 1;
+                break;
+            }
+            if (retflag)
+                break;
+            freeheap();
+            oldval = lastval;
+        }
     cmdpop();
     popheap();
     loops--;

-- 
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: [falk@debian.org: Bug#107528: "while do done" gets into uninterruptable loop]

[Clint Adams]

> The patch below looks stupid (and it may well be), but it's the easiest
> I can think of.  I think I better not commit this until I get replies.
> (Most of it is re-indentation of that loop, there's just a tiny bit of
> added code at the top.)

This seems to work;  I don't have any objection to its committal.

Re: [falk@debian.org: Bug#107528: "while do done" gets into uninterruptable loop]

[Sven Wischnowsky]

Clint Adams wrote:

> > The patch below looks stupid (and it may well be), but it's the easiest
> > I can think of.  I think I better not commit this until I get replies.
> > (Most of it is re-indentation of that loop, there's just a tiny bit of
> > added code at the top.)
> 
> This seems to work;  I don't have any objection to its committal.

Nor have I. And since noone else complained I'm just going to commit
it.


Bye
  Sven

Index: Src/loop.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/loop.c,v
retrieving revision 1.7
diff -u -r1.7 loop.c
--- Src/loop.c	23 Oct 2001 16:17:05 -0000	1.7
+++ Src/loop.c	25 Feb 2002 09:18:48 -0000
@@ -380,37 +380,53 @@
     cmdpush(isuntil ? CS_UNTIL : CS_WHILE);
     loops++;
     loop = state->pc;
-    for (;;) {
-	state->pc = loop;
-	noerrexit = 1;
-	execlist(state, 1, 0);
-	noerrexit = olderrexit;
-	if (!((lastval == 0) ^ isuntil)) {
-	    if (breaks)
-		breaks--;
-	    lastval = oldval;
-	    break;
-	}
-	if (retflag) {
-	    lastval = oldval;
-	    break;
-	}
-	execlist(state, 1, 0);
-	if (breaks) {
-	    breaks--;
-	    if (breaks || !contflag)
-		break;
-	    contflag = 0;
-	}
-	if (errflag) {
-	    lastval = 1;
-	    break;
-	}
-	if (retflag)
-	    break;
-	freeheap();
-	oldval = lastval;
-    }
+
+    if (loop[0] == WC_END && loop[1] == WC_END) {
+
+        /* This is an empty loop.  Make sure the signal handler sets the
+        * flags and then just wait for someone hitting ^C. */
+
+        int old_simple_pline = simple_pline;
+
+        simple_pline = 1;
+
+        while (!breaks)
+            ;
+        breaks--;
+
+        simple_pline = old_simple_pline;
+    } else
+        for (;;) {
+            state->pc = loop;
+            noerrexit = 1;
+            execlist(state, 1, 0);
+            noerrexit = olderrexit;
+            if (!((lastval == 0) ^ isuntil)) {
+                if (breaks)
+                    breaks--;
+                lastval = oldval;
+                break;
+            }
+            if (retflag) {
+                lastval = oldval;
+                break;
+            }
+            execlist(state, 1, 0);
+            if (breaks) {
+                breaks--;
+                if (breaks || !contflag)
+                    break;
+                contflag = 0;
+            }
+            if (errflag) {
+                lastval = 1;
+                break;
+            }
+            if (retflag)
+                break;
+            freeheap();
+            oldval = lastval;
+        }
     cmdpop();
     popheap();
     loops--;

-- 
Sven Wischnowsky                          wischnow@berkom.de