From: Jilles Tjoelker <jilles@stack.nl>
To: dash@vger.kernel.org
Subject: Re: trap bug in recent versions of dash
Date: Sun, 15 Aug 2010 22:05:51 +0200 [thread overview]
Message-ID: <20100815200551.GA19150@stack.nl> (raw)
In-Reply-To: <20100811080616.GA18303@wopr.local.invalid>
On Wed, Aug 11, 2010 at 10:06:16AM +0200, Guido Berhoerster wrote:
> with the latest git version of dash trap actions are not
> evaluated in the context of a function.
I think dotrap()'s return value should be removed or at least ignored.
An "evalskip" (break/continue/return-function/return-file) should never
lead to an immediate exit. I'm not supplying a patch because I am not
entirely sure about the side effects this could have.
Related, there may be a bug with SKIPFILE. This constant is never tested
against, and this makes 'break' inside a dot script behave strangely.
Example (this must be saved to a file, as it sources itself):
if [ "$1" != nested ]; then
while :; do
set -- nested
. "$0"
echo bad2
exit 2
done
exit 0
fi
break
echo bad1
exit 1
> The following script demonstrates the bug:
> ----8<----
> read_timeout () {
> saved_traps="$(trap)"
This does not work in dash, it always returns an empty string because
trap is evaluated in a subshell.
Some other ash variants do not evaluate most command substitutions
containing only a single builtin in a subshell, but this was removed in
NetBSD sh and dash because such commands can affect the shell in wrong
ways (e.g. $(exit 1) causes FreeBSD sh to exit).
A recent or proposed POSIX interpretation has said that $(trap) should
work, and that this may be done by treating a lone trap command
specially or by having trap in a subshell output the parent's traps
until a trap has been set in the subshell. To help the former case,
stuff like TRAP=trap; y=$($TRAP) is not required to work.
A problem in the script is that it does not handle TERM set to the
default action properly, as this is not included in trap's output.
> trap 'printf "timed out\n"; eval "${saved_traps}"; return' TERM
> ( sleep $1; kill -TERM $$ ) >/dev/null 2>&1 &
For portability, I recommend braces
{ sleep $1; kill -TERM $$; } >/dev/null 2>&1 &
Some shells do not treat a background subshell specially and fork twice,
which would cause the wrong process to be killed below.
> timer_pid=$!
> read $2
> kill $timer_pid 2>/dev/null
eval "${saved_traps}" is missing here.
> }
> read_timeout 5 value
> printf "read \"%s\"\n" "${value:=default}"
> ---->8----
> The return statement in the trap inside the read_timeout function
> does not return from the function but rather exits the script.
> With dash 0.5.5.1 it works as expected.
--
Jilles Tjoelker
next prev parent reply other threads:[~2010-08-15 20:05 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-11 8:06 trap bug in recent versions of dash Guido Berhoerster
2010-08-15 20:05 ` Jilles Tjoelker [this message]
2010-08-16 11:52 ` Guido Berhoerster
2010-08-22 22:32 ` Jilles Tjoelker
2010-08-23 10:40 ` Guido Berhoerster
2010-08-23 20:11 ` Jilles Tjoelker
2010-11-28 7:20 ` Herbert Xu
2010-11-28 7:19 ` Herbert Xu
2010-11-28 8:50 ` Guido Berhoerster
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=20100815200551.GA19150@stack.nl \
--to=jilles@stack.nl \
--cc=dash@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.