From owner-svn-src-stable@FreeBSD.ORG Sat Oct 23 21:20:26 2010 Return-Path: Delivered-To: svn-src-stable@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B8DE81065673; Sat, 23 Oct 2010 21:20:26 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay04.stack.nl [IPv6:2001:610:1108:5010::107]) by mx1.freebsd.org (Postfix) with ESMTP id 7BBB78FC15; Sat, 23 Oct 2010 21:20:26 +0000 (UTC) Received: from turtle.stack.nl (turtle.stack.nl [IPv6:2001:610:1108:5010::132]) by mx1.stack.nl (Postfix) with ESMTP id 38C011DD732; Sat, 23 Oct 2010 23:20:24 +0200 (CEST) Received: by turtle.stack.nl (Postfix, from userid 1677) id 2CA1317280; Sat, 23 Oct 2010 23:20:24 +0200 (CEST) Date: Sat, 23 Oct 2010 23:20:24 +0200 From: Jilles Tjoelker To: "David E. O'Brien" Message-ID: <20101023212023.GA10891@stack.nl> References: <201010182310.o9INAWPU089616@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201010182310.o9INAWPU089616@svn.freebsd.org> User-Agent: Mutt/1.5.20 (2009-06-14) 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 X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 23 Oct 2010 21:20:26 -0000 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