Date: Sat, 23 Oct 2010 23:20:24 +0200 From: Jilles Tjoelker <jilles@stack.nl> To: "David E. O'Brien" <obrien@FreeBSD.org> Cc: svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-8@freebsd.org Subject: Re: svn commit: r214045 - in stable/8/tools/regression/bin/sh: builtins errors execution expansion set-e Message-ID: <20101023212023.GA10891@stack.nl> In-Reply-To: <201010182310.o9INAWPU089616@svn.freebsd.org> References: <201010182310.o9INAWPU089616@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Oct 18, 2010 at 11:10:32PM +0000, David E. O'Brien wrote: > Author: obrien > Date: Mon Oct 18 23:10:32 2010 > New Revision: 214045 > URL: http://svn.freebsd.org/changeset/base/214045 > > Log: > MFC: > r204801: make sure to popredir() even if a special builtin caused an error > r204802: make sure to popredir() even if a function caused an error > Copied: stable/8/tools/regression/bin/sh/builtins/command10.0 (from r204802, head/tools/regression/bin/sh/builtins/command10.0) > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ stable/8/tools/regression/bin/sh/builtins/command10.0 Mon Oct 18 23:10:32 2010 (r214045, copy of r204802, head/tools/regression/bin/sh/builtins/command10.0) > @@ -0,0 +1,14 @@ > +# $FreeBSD$ > + > +failures=0 > + > +check() { > + if ! eval "[ $* ]"; then > + echo "Failed: $*" > + : $((failures += 1)) > + fi > +} > + > +check '"$(f() { shift x; }; { command eval f 2>/dev/null; } >/dev/null; echo hi)" = hi' > + > +exit $((failures > 0)) > Copied: stable/8/tools/regression/bin/sh/builtins/command9.0 (from r204801, head/tools/regression/bin/sh/builtins/command9.0) > ============================================================================== > --- /dev/null 00:00:00 1970 (empty, because file is newly added) > +++ stable/8/tools/regression/bin/sh/builtins/command9.0 Mon Oct 18 23:10:32 2010 (r214045, copy of r204801, head/tools/regression/bin/sh/builtins/command9.0) > @@ -0,0 +1,14 @@ > +# $FreeBSD$ > + > +failures=0 > + > +check() { > + if ! eval "[ $* ]"; then > + echo "Failed: $*" > + : $((failures += 1)) > + fi > +} > + > +check '"$({ command eval shift x 2>/dev/null; } >/dev/null; echo hi)" = hi' > + > +exit $((failures > 0)) These work on stable/8, but for the wrong reason. In stable/8, 'command' does not allow executing builtin utilities, and therefore 'eval' cannot be found. This is a different error than the expected passing of a non-number to 'shift', and does not allow testing for the bug this is supposed to check for. Some of the other tests for these kinds of bugs use fifos and 'fc', which also work in older sh (from the point when POSIX-style special builtins were implemented; before that, 'fc' errors were fatal, probably for this reason). However, when I allowed 'command' to execute builtin utilities, I started to use 'command eval' because it makes the tests more readable and easier to write. Demonstration of the bug in stable/8: $ sh $ shift x shift: Illegal number: x $ echo $( { fc -e :; } >/dev/null; echo hi) shift: Illegal number: x $ "hi" should be printed, and is printed correctly if the procedure is repeated without the >/dev/null redirection. Because this does not cause any extra failures, I don't think it is necessary to revert it. However, it seems unusual to MFC a test for a bugfix without also MFCing the bugfix. -- Jilles Tjoelker
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101023212023.GA10891>