All of lore.kernel.org
 help / color / mirror / Atom feed
* 'continue' does not work in files sourced with dotcmd
@ 2011-02-26 18:23 Taylan Ulrich B.
  2011-07-07  3:37 ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Taylan Ulrich B. @ 2011-02-26 18:23 UTC (permalink / raw)
  To: dash

Using latest dash from git.
Transcript:

$ echo continue > foo
$ for i in 1; do . ./foo; echo foobar; done
foobar
$

By the way you guys should look into the Debian bugtracker for their
dash package, if you don't already; most of the bugs there are
probably upstream.

Dash is awesome.
Regards,
Taylan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-02-26 18:23 'continue' does not work in files sourced with dotcmd Taylan Ulrich B.
@ 2011-07-07  3:37 ` Herbert Xu
  2011-07-07 13:26   ` Eric Blake
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2011-07-07  3:37 UTC (permalink / raw)
  To: Taylan Ulrich B.; +Cc: dash

On Sat, Feb 26, 2011 at 06:23:54PM +0000, Taylan Ulrich B. wrote:
> Using latest dash from git.
> Transcript:
> 
> $ echo continue > foo
> $ for i in 1; do . ./foo; echo foobar; done
> foobar
> $

While this does look inconsistent, this behaviour is not confined
to dash.  pdksh also does the same thing.

So I'm not particularly keen on changing this.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-07-07  3:37 ` Herbert Xu
@ 2011-07-07 13:26   ` Eric Blake
  2011-07-07 14:46     ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2011-07-07 13:26 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Taylan Ulrich B., dash

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

On 07/06/2011 09:37 PM, Herbert Xu wrote:
> On Sat, Feb 26, 2011 at 06:23:54PM +0000, Taylan Ulrich B. wrote:
>> Using latest dash from git.
>> Transcript:
>>
>> $ echo continue > foo
>> $ for i in 1; do . ./foo; echo foobar; done
>> foobar
>> $
> 
> While this does look inconsistent, this behaviour is not confined
> to dash.  pdksh also does the same thing.

I've asked on the Austin Group whether this is a hole in POSIX, since
the standard appears to be silent on whether a for loop around a dot
script is visible to commands within the dot script.

> 
> So I'm not particularly keen on changing this.

Unfortunately, I argue that dash DOES have a bug.  Compare the zsh behavior:

$ zsh -c 'emulate sh; printf "continue\\necho \$?" > f; for i in 1; do .
./f; echo "$? hi"; done; rm f'
./f:continue:1: not in while, until, select, or repeat loop
1 hi

zsh rejected the continue statement (no containing loop within the
current scoping of the dot script), and as a result also aborted the
overall dot script - but in doing so, it properly emitted an error message.

Unless the Austin Group rules otherwise, I'm okay with dash rejecting
the continue command and aborting the dot script - but only on the
condition that the error is diagnosed, rather than silently papered
over.  But it may be worth waiting for the Austin Group ruling, to see
if the bash/ksh behavior of continuing on to the loop that is enclosing
the dot script is required, or merely one possible alternative, before
changing dash behavior (that is, no point wiring up an error message
unless we know that issuing an error message complies with the standard).

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-07-07 13:26   ` Eric Blake
@ 2011-07-07 14:46     ` Herbert Xu
  2011-07-07 15:18       ` Eric Blake
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2011-07-07 14:46 UTC (permalink / raw)
  To: Eric Blake; +Cc: Taylan Ulrich B., dash

On Thu, Jul 07, 2011 at 07:26:50AM -0600, Eric Blake wrote:
> 
> zsh rejected the continue statement (no containing loop within the
> current scoping of the dot script), and as a result also aborted the
> overall dot script - but in doing so, it properly emitted an error message.

dash does not warn in this case because it never prints a warning
on continue anywhere.

This behaviour (not warning) is consistent with that of ksh.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-07-07 14:46     ` Herbert Xu
@ 2011-07-07 15:18       ` Eric Blake
  2011-07-08  8:09         ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2011-07-07 15:18 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Taylan Ulrich B., dash, zsh-workers

[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]

[adding zsh list]

On 07/07/2011 08:46 AM, Herbert Xu wrote:
> On Thu, Jul 07, 2011 at 07:26:50AM -0600, Eric Blake wrote:
>>
>> zsh rejected the continue statement (no containing loop within the
>> current scoping of the dot script), and as a result also aborted the
>> overall dot script - but in doing so, it properly emitted an error message.
> 
> dash does not warn in this case because it never prints a warning
> on continue anywhere.
> 
> This behaviour (not warning) is consistent with that of ksh.

Interesting!  And bash indeed behaves differently: in POSIX mode it is
silent, while in bash mode it issues a warning but does not affect exit
status - in line with the fact that in POSIX mode, bash obeys the rule
that if an application is diagnosing something to stderr, then the exit
status should be non-zero (and warnings don't affect exit status, hence
warnings should not be issued in POSIX mode).

Zsh is treating it as a syntax error, which is different from all the
other shells:

$ bash -c 'continue; echo $?'
bash: line 0: continue: only meaningful in a `for', `while', or `until' loop
0
$ bash --posix -c 'continue; echo $?'
0
$ dash -c 'continue; echo $?'
0
$ ksh -c 'continue; echo $?'
0
$ zsh -c 'emulate sh; continue; echo $?'
zsh:continue:1: not in while, until, select, or repeat loop
$ echo $?
1

POSIX states "If n is greater than the number of enclosing loops, the
outermost enclosing loop shall be exited."  But this is admittedly
silent on the behavior when there is no outermost enclosing loop, so I'm
not sure whether zsh treating this as a syntax error is appropriate.

Meanwhile, I still have to wonder about dash behavior - if dash is _not_
treating continue as a syntax error, then why is it aborting the dot
script?  Compare:

$ dash -c 'continue
echo $?'
0
$ dash -c 'printf "continue\\necho 0.\$?\\n" > f; for i in 1; do . ./f;
echo 1.$?; done; echo 2.$?'
1.0
2.0

In isolation, execution after an un-nested continue resumed with the
next statement, but in the dot script, execution after the "un-nested"
continue (at least, from the dot script perspective) aborted the dot
script, but without affecting exit status.

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-07-07 15:18       ` Eric Blake
@ 2011-07-08  8:09         ` Herbert Xu
  2011-07-09 13:07           ` Jilles Tjoelker
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2011-07-08  8:09 UTC (permalink / raw)
  To: Eric Blake; +Cc: Taylan Ulrich B., dash, zsh-workers

On Thu, Jul 07, 2011 at 09:18:12AM -0600, Eric Blake wrote:
>
> Meanwhile, I still have to wonder about dash behavior - if dash is _not_
> treating continue as a syntax error, then why is it aborting the dot
> script?  Compare:

This is a consequence of the way continue/break/return is implemented
in dash.

As this construct cannot be used portably anyway, I'm not inclined to
make any changes unless you can do it without increasing the code
size.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-07-08  8:09         ` Herbert Xu
@ 2011-07-09 13:07           ` Jilles Tjoelker
  2011-07-09 14:07             ` Herbert Xu
  0 siblings, 1 reply; 9+ messages in thread
From: Jilles Tjoelker @ 2011-07-09 13:07 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Eric Blake, Taylan Ulrich B., dash

On Fri, Jul 08, 2011 at 04:09:36PM +0800, Herbert Xu wrote:
> On Thu, Jul 07, 2011 at 09:18:12AM -0600, Eric Blake wrote:
> > Meanwhile, I still have to wonder about dash behavior - if dash is _not_
> > treating continue as a syntax error, then why is it aborting the dot
> > script?  Compare:

> This is a consequence of the way continue/break/return is implemented
> in dash.

> As this construct cannot be used portably anyway, I'm not inclined to
> make any changes unless you can do it without increasing the code
> size.

It is not a hard consequence, the behaviour can be fixed fairly easily.

I noticed a similar problem in FreeBSD sh and fixed it in head in August
2010, Subversion r211349.

A testcase is (note that this must be saved to a named file because it
sources itself):
####
# $FreeBSD: head/tools/regression/bin/sh/builtins/break1.0 211349 2010-08-15 21:06:53Z jilles $

if [ "$1" != nested ]; then
	while :; do
		set -- nested
		. "$0"
		echo bad2
		exit 2
	done
	exit 0
fi
# To trigger the bug, the following commands must be at the top level,
# with newlines in between.
break
echo bad1
exit 1
####

Given that quite a few shells pass this test, I consider it prudent to
make it work. It is not unlikely that someone somewhere has written a
script that depends on it, and treating the break or continue as a
return is rather surprising in a negative way. (Similar for break or
continue in eval or trap, though I think those work in dash.)

A fix for dash is below. The dash code is broken in a different way than
the FreeBSD sh code was, but the patched code is pretty much the same.
This makes the above test work and does not change the outcome of any
other tests in the FreeBSD sh testsuite.

diff --git a/src/main.c b/src/main.c
index af987c6..cdd91e2 100644
--- a/src/main.c
+++ b/src/main.c
@@ -242,7 +242,8 @@ cmdloop(int top)
 
 		skip = evalskip;
 		if (skip) {
-			evalskip = 0;
+			if (skip == SKIPFILE)
+				evalskip = 0;
 			break;
 		}
 	}

Using
	evalskip &= ~SKIPFILE;
instead of
	if (skip == SKIPFILE)
		evalskip = 0;
should also work and might generate shorter code.

-- 
Jilles Tjoelker

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-07-09 13:07           ` Jilles Tjoelker
@ 2011-07-09 14:07             ` Herbert Xu
  2011-07-10 19:03               ` Jilles Tjoelker
  0 siblings, 1 reply; 9+ messages in thread
From: Herbert Xu @ 2011-07-09 14:07 UTC (permalink / raw)
  To: Jilles Tjoelker; +Cc: Eric Blake, Taylan Ulrich B., dash

On Sat, Jul 09, 2011 at 03:07:04PM +0200, Jilles Tjoelker wrote:
> 
> A fix for dash is below. The dash code is broken in a different way than
> the FreeBSD sh code was, but the patched code is pretty much the same.
> This makes the above test work and does not change the outcome of any
> other tests in the FreeBSD sh testsuite.

You're right.  But I think your patch may introduce a problem
with a return statement inside a dot script.  That should not
have an effect after exiting the script.

Anyway, the following patch based on your idea should fix the
problem.

commit 4f7e206782675b548565ca2bc82bc8c262a0f20e
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Sat Jul 9 22:05:22 2011 +0800

    [BUILTIN] Merge SKIPFUNC/SKIPFILE and only clear SKIPFUNC when leaving dotcmd
    
    Currently upon leaving a dotcmd the evalskip state is reset so
    if a continue/break statement is used within a dot script it would
    have no effect outside of the dot script.
    
    This is inconsistent with other shells.
    
    This patch is based on one by Jilles Tjoelker and only clears
    SKIPFUNC when leaving a dot script.  As a result continue/break
    will remain in effect.
    
    It also merges SKIPFUNC/SKIPFILE as they have no practical difference.
    
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/ChangeLog b/ChangeLog
index d3a4acf..dfab8d1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2011-07-09  Herbert Xu <herbert@gondor.apana.org.au>
+
+	* Merge SKIPFUNC/SKIPFILE and only clear SKIPFUNC when leaving dotcmd.
+
 2011-07-08  Herbert Xu <herbert@gondor.apana.org.au>
 
 	* Release 0.5.7.
diff --git a/src/eval.c b/src/eval.c
index 95d30f4..c7358a6 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1039,7 +1039,7 @@ returncmd(int argc, char **argv)
 	 * If called outside a function, do what ksh does;
 	 * skip the rest of the file.
 	 */
-	evalskip = funcline ? SKIPFUNC : SKIPFILE;
+	evalskip = SKIPFUNC;
 	return argv[1] ? number(argv[1]) : exitstatus;
 }
 
diff --git a/src/eval.h b/src/eval.h
index 4e4de30..dc8acd2 100644
--- a/src/eval.h
+++ b/src/eval.h
@@ -61,4 +61,3 @@ extern int evalskip;
 #define SKIPBREAK	(1 << 0)
 #define SKIPCONT	(1 << 1)
 #define SKIPFUNC	(1 << 2)
-#define SKIPFILE	(1 << 3)
diff --git a/src/main.c b/src/main.c
index af987c6..f79ad7d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -242,7 +242,7 @@ cmdloop(int top)
 
 		skip = evalskip;
 		if (skip) {
-			evalskip = 0;
+			evalskip &= ~SKIPFUNC;
 			break;
 		}
 	}

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: 'continue' does not work in files sourced with dotcmd
  2011-07-09 14:07             ` Herbert Xu
@ 2011-07-10 19:03               ` Jilles Tjoelker
  0 siblings, 0 replies; 9+ messages in thread
From: Jilles Tjoelker @ 2011-07-10 19:03 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Eric Blake, Taylan Ulrich B., dash

On Sat, Jul 09, 2011 at 10:07:30PM +0800, Herbert Xu wrote:
> On Sat, Jul 09, 2011 at 03:07:04PM +0200, Jilles Tjoelker wrote:
> > A fix for dash is below. The dash code is broken in a different way than
> > the FreeBSD sh code was, but the patched code is pretty much the same.
> > This makes the above test work and does not change the outcome of any
> > other tests in the FreeBSD sh testsuite.

> You're right.  But I think your patch may introduce a problem
> with a return statement inside a dot script.  That should not
> have an effect after exiting the script.

Interesting. Dash has been returning from the closest scope (function or
sourced script) for a while, but the SKIPFUNC/SKIPFILE distinction and
the comment in eval.c returncmd()

]  	/*
]  	 * If called outside a function, do what ksh does;
]  	 * skip the rest of the file.
]  	 */

still gave me the impression that it behaved like older ash (also in
FreeBSD and NetBSD), trying to be bug-compatible with the Bourne shell
by having 'return' return from a function, if any, and only return from
a dot script if there is no function (because the Bourne shell gives an
error in that case).

It may be better to name the constant SKIPRETURN rather than SKIPFUNC.

> Anyway, the following patch based on your idea should fix the
> problem.

> commit 4f7e206782675b548565ca2bc82bc8c262a0f20e
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date:   Sat Jul 9 22:05:22 2011 +0800
> 
>     [BUILTIN] Merge SKIPFUNC/SKIPFILE and only clear SKIPFUNC when leaving dotcmd

Yes, this works too.

-- 
Jilles Tjoelker

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-07-10 19:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-26 18:23 'continue' does not work in files sourced with dotcmd Taylan Ulrich B.
2011-07-07  3:37 ` Herbert Xu
2011-07-07 13:26   ` Eric Blake
2011-07-07 14:46     ` Herbert Xu
2011-07-07 15:18       ` Eric Blake
2011-07-08  8:09         ` Herbert Xu
2011-07-09 13:07           ` Jilles Tjoelker
2011-07-09 14:07             ` Herbert Xu
2011-07-10 19:03               ` Jilles Tjoelker

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.