Skip site navigation (1)Skip section navigation (2)
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>