Dash Archive mirror
 help / color / mirror / Atom feed
From: Harald van Dijk <harald@gigawatt.nl>
To: Vitaly Sinilin <vs@kp4.ru>, dash@vger.kernel.org
Subject: Re: [BUG] Redirection bug in subshell's EXIT trap
Date: Thu, 19 Oct 2017 23:57:46 +0200	[thread overview]
Message-ID: <8790d678-ee72-fdb5-750b-d2bb64cf2ec2@gigawatt.nl> (raw)
In-Reply-To: <20171019174410.GA22649@ghost.hq.kp4.ru>

On 19/10/17 19:44, Vitaly Sinilin wrote:
> Hi,
> 
> I've recently written a quite unusual script and faced a strange dash
> behavior that seems not POSIX-ly correct to me.
> 
> Here is the script with a lot of debugging extra output.
> 
> (I am sorry about posting a pretty lengthy script, but I feel like a
> shorten version will look just insane without the context.)

A shortened version is easier to debug:

   dash -c 'f()(trap cat EXIT);f<&-'

This is supposed to print error messages. With dash, it doesn't.

> The problem here is that although getch is run in a subshell and it
> got FD#4 as stdin it somehow has parent's stdin in its EXIT trap.
> 
> POSIX says: "The environment in which the shell executes a trap on
> EXIT shall be identical to the environment immediately after the
> last command executed before the trap on EXIT was taken."

Yes, it is a bug. The problem is that dash, just before checking whether 
to execute the EXIT handler, has already reset everything to continue 
parsing and evaluating the next command. Which is not supposed to 
happen, never going to happen, and messes up the execution of the handler.

This resetting is done by the reset() function, and although the simple 
fix would be to move its call after exitshell(), the old ChangeLog shows 
that it was originally (about 15 years ago) the other way around and 
intentionally switched to how it is now, because some of what reset() 
does is necessary even here:

>  -- Herbert Xu <herbert@debian.org>  Sat, 26 Oct 2002 21:28:33 +1000
> 
> dash (0.4.2) unstable; urgency=low
> 
>   [...]
>   * Call reset() before exitshell() is called.  This fixes the bug where
>     returning an error from a function running under set -e caused the exit
>     trap to be taken with evalskip set.

Aside from the issue with redirections, the current order has other 
problems too:

   dash -c 'f()(local cmd="echo good";trap \$cmd EXIT);cmd="echo bad";f'

This is supposed to print "good". With dash, it prints "bad". With the 
call to reset() moved after exitshell(), it prints "good".

I suspect reset() needs to be split into two separate functions, but it 
may be a bit tricky to determine exactly what is supposed to go where.

Cheers,
Harald van Dijk

      reply	other threads:[~2017-10-19 22:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-19 17:44 [BUG] Redirection bug in subshell's EXIT trap Vitaly Sinilin
2017-10-19 21:57 ` Harald van Dijk [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8790d678-ee72-fdb5-750b-d2bb64cf2ec2@gigawatt.nl \
    --to=harald@gigawatt.nl \
    --cc=dash@vger.kernel.org \
    --cc=vs@kp4.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).