Re: trap bug in recent versions of dash

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxxx>]

On Wed, Aug 11, 2010 at 08:06:16AM +0000, Guido Berhoerster wrote:
> Hello,
> 
> with the latest git version of dash trap actions are not
> evaluated in the context of a function.
> 
> The following script demonstrates the bug:
> ----8<----
> read_timeout () {
>     saved_traps="$(trap)"
>     trap 'printf "timed out\n"; eval "${saved_traps}"; return' TERM
>     ( sleep $1; kill -TERM $$ ) >/dev/null 2>&1 &
>     timer_pid=$!
>     read $2
>     kill $timer_pid 2>/dev/null
> }
> 
> 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.

Indeed this was a regression caused by the SKIPEVAL removal.

This patch should fix it.

commit faefce1ebe585366b1458031f2c1f723dc630def
Author: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Date:   Sun Nov 28 15:17:45 2010 +0800

    [EVAL] Fixed trap/return regression due to SKIPEVAL removal
    
    On Wed, Aug 11, 2010 at 08:06:16AM +0000, Guido Berhoerster wrote:
    >
    > with the latest git version of dash trap actions are not
    > evaluated in the context of a function.
    >
    > The following script demonstrates the bug:
    > ----8<----
    > read_timeout () {
    >     saved_traps="$(trap)"
    >     trap 'printf "timed out\n"; eval "${saved_traps}"; return' TERM
    >     ( sleep $1; kill -TERM $$ ) >/dev/null 2>&1 &
    >     timer_pid=$!
    >     read $2
    >     kill $timer_pid 2>/dev/null
    > }
    >
    > 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.
    
    This bug was caused by the SKIPEVAL removal.  When the SKIPEVAL
    hack was added to improve set -e support in traps, dotrap was
    changed to return whether set -e was detected.  After the removal
    of SKIPEVAL, set -e is now handled through exraise.
    
    However, dotrap still returned a value which is now incorrectly
    used to trigger an exraise.
    
    This patch removes the vestigial link between dotrap and exraise.
    
    Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

diff --git a/ChangeLog b/ChangeLog
index 5ace9ff..f148ef6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2010-11-28  Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
+
+	* Fixed trap/return regression due to SKIPEVAL removal.
+
 2010-10-18  Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
 
 	* Fix ifsfirst/ifslastp leak in casematch.
diff --git a/src/eval.c b/src/eval.c
index b966749..ea4afc6 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -304,10 +304,16 @@ setstatus:
 		break;
 	}
 out:
-	if ((checkexit & exitstatus) ||
-	    (pendingsigs && dotrap()) ||
-	    (flags & EV_EXIT))
+	if (checkexit & exitstatus)
+		goto exexit;
+
+	if (pendingsigs)
+		dotrap();
+
+	if (flags & EV_EXIT) {
+exexit:
 		exraise(EXEXIT);
+	}
 }
 
 
diff --git a/src/trap.c b/src/trap.c
index 7bd60ab..3d28485 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -309,8 +309,7 @@ onsig(int signo)
  * handlers while we are executing a trap handler.
  */
 
-int
-dotrap(void)
+void dotrap(void)
 {
 	char *p;
 	char *q;
@@ -332,10 +331,8 @@ dotrap(void)
 		evalstring(p, 0);
 		exitstatus = savestatus;
 		if (evalskip)
-			return evalskip;
+			break;
 	}
-
-	return 0;
 }
 
 
diff --git a/src/trap.h b/src/trap.h
index bdd7944..7573fd7 100644
--- a/src/trap.h
+++ b/src/trap.h
@@ -46,7 +46,7 @@ void clear_traps(void);
 void setsignal(int);
 void ignoresig(int);
 void onsig(int);
-int dotrap(void);
+void dotrap(void);
 void setinteractive(int);
 void exitshell(void) __attribute__((__noreturn__));
 int decode_signal(const char *, int);

Thanks,
-- 
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html