/bin/sh compatibility issue

[Johannes Hofmann]

The commited patch fixes the problem.
Thanks,
Johannes

YONETANI Tomokazu <qhwt+dfly at xxxxxxxxxx> wrote:
> On Wed, May 10, 2006 at 05:59:05PM +0100, Steve O'Hara-Smith wrote:
>> On Wed, 10 May 2006 17:49:38 +0200
>> joerg at xxxxxxxxxxxxxxxxx wrote:
>> 
>> > On Wed, May 10, 2006 at 03:39:53PM +0000, Johannes Hofmann wrote:
>> > > The following shell script outputs "false1" with all shells I tried
>> > > but with /bin/sh on DragonFly. Is this a known incompatibility?
>> > 
>> > I have to read the POSIX definitions, but this doesn't look very wrong
>> > at least. Actually, I can understand this behaviour at the very least.
>> 
>>       It doesn't seem to be consistent with the description of -e
>> in the sh manpage.
>> 
>> -e errexit
>>   Exit immediately if any untested command fails in non-interactive mode.
>>   The exit status of a command is considered to be explicitly tested if the
>>   command is used to control an if, elif, while, or until; or if the command
>>   is the left hand operand of an ``&&'' or ``||'' operator.
>> 
>>       I've quoted the script with added comments.
>> 
>> #!/bin/sh
>> set -e
>> if true; then
>>    false && echo "huh?"   # This shouldn't exit because false is LHS of &&
>> fi
>> echo "false1"
>> false   # This should exit because false is not explicitly tested
>> echo "false2"
> 
> /bin/sh in FreeBSD-4.x has the same problem, but not in FreeBSD-5.x or later.
> The revisions 1.38,1.41 of eval.c fixes this behavior, but hasn't been MFC'ed
> to 4.x(therefore we don't have it either).  The commitlog recommends
> that it should be MFC'ed after longer than usual, but I think two and a half
> year is long enough :)
> Revisions 1.44-1.46 also seem to be -e related fixes.
> 
> diff -r 89a6bb686df8 bin/sh/eval.c
> --- bin/sh/eval.c       Wed May 03 06:28:01 2006 +0000
> +++ bin/sh/eval.c       Thu May 11 11:45:37 2006 +0900
> @@ -202,7 +202,6 @@ evaltree(union node *n, int flags)
>        case NAND:
>                evaltree(n->nbinary.ch1, EV_TESTED);
>                if (evalskip || exitstatus != 0) {
> -                       flags |= EV_TESTED;
>                        goto out;
>                }
>                evaltree(n->nbinary.ch2, flags);
> @@ -243,25 +242,9 @@ evaltree(union node *n, int flags)
>                break;
>        case NFOR:
>                evalfor(n);
> -               /*
> -                * The 'for' command does not set exitstatus, so the value
> -                * now in exitstatus is from the last command executed in
> -                * the 'for' loop.  That exit value had been tested (wrt
> -                * 'sh -e' checking) while processing that command, and
> -                * it should not be re-tested here.
> -                */
> -               flags |= EV_TESTED;
>                break;
>        case NCASE:
>                evalcase(n, flags);
> -               /*
> -                * The 'case' command does not set exitstatus, so the value
> -                * now in exitstatus is from the last command executed in
> -                * the 'case' block.  That exit value had been tested (wrt
> -                * 'sh -e' checking) while processing that command, and
> -                * it should not be re-tested here.
> -                */
> -               flags |= EV_TESTED;
>                break;
>        case NDEFUN:
>                defun(n->narg.text, n->narg.next);
> @@ -286,14 +269,9 @@ out:
> out:
>        if (pendingsigs)
>                dotrap();
> -       /*
> -        * XXX - Like "!(n->type == NSEMI)", more types will probably
> -        * need to be excluded from this test. It's probably better
> -        * to set or unset EV_TESTED in the loop above than to bloat
> -        * the conditional here.
> -        */
>        if ((flags & EV_EXIT) || (eflag && exitstatus 
> -           && !(flags & EV_TESTED) && !(n->type == NSEMI)))
> +           && !(flags & EV_TESTED) && (n->type == NCMD ||
> +           n->type == NSUBSHELL)))
>                exitshell(exitstatus);
> }
>