[prev in list] [next in list] [prev in thread] [next in thread] 

List:       dash
Subject:    Re: [v2 PATCH] eval: Only restore exit status on exit/return
From:       Harald van Dijk <harald () gigawatt ! nl>
Date:       2018-12-14 10:54:27
Message-ID: 1ea9e4eb-9a8a-fc66-8377-743149c36f13 () gigawatt ! nl
[Download RAW message or body]

On 14/12/2018 10:07, Harald van Dijk wrote:
> On 14/12/2018 09:47, Herbert Xu wrote:
>> On Fri, Dec 14, 2018 at 09:37:09AM +0000, Harald van Dijk wrote:
>>>
>>> Still, that works as well, doesn't it? The control flow is basically the
>>> same so the logic in my other message applies. returncmd() doesn't use
>>> exceptions, so you get to dotrap() which already resets exitstatus if
>>> necessary.
>>
>> returncmd itself doesn't jump up but it may be part of a subshell
>> which would execute a jump to the top as part of the EV_EXIT
>> optimisation.  That's where you'd need to restore the exit status.
> 
> Ah, okay, so for something like
> 
>    trap 'false; (return); echo $?' EXIT
> 
> you want to remember that it's executed as part of a trap action and 
> print 0, not 1.
> 
> I actually want that to print 1, so for me it's not a problem. However, 
> my patch can be trivially modified to handle that: just have returncmd() 
> check savestatus the same way exitcmd() does. I think that also allows 
> merging SKIPFUNC and SKIPFUNCDEF, since you can then assume exitstatus 
> was always set appropriately, so it allows for some further reduction in 
> code.

I think this is needed regardless to fix a bug as well:

   trap 'f() { false; return; }; f; echo $?' EXIT

Other shells are evenly divided, but POSIX clearly requires this to 
print 0 (which is what bash/ksh/mksh/yash do): calling a function does 
not reset any trap information, so this return statement must be 
considered to execute in a trap action. It currently prints 1 (also seen 
in bosh/pdksh/posh/zsh). With my suggested changes, it prints 0.
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic