All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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.