hush: do not reset to default "" traps in subshell

function                                             old     new   delta
reset_traps_to_defaults                              164     211     +47
builtin_umask                                        123     121      -2

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

shell/hush.c

@@ -1112,13 +1112,10 @@ static void restore_G_args(save_arg_t *sv, char **argv)
  * are to count SIGCHLDs
  * and to restore tty pgrp on signal-induced exit.
  *
- * TODO compat:
+ * Note 2 (compat):
  * Standard says "When a subshell is entered, traps that are not being ignored
  * are set to the default actions". bash interprets it so that traps which
- * are set to "" (ignore) are NOT reset to defaults. We reset them to default.
+ * are set to "" (ignore) are NOT reset to defaults. We do the same.
- * bash example:
- * # trap '' SYS; (bash -c 'kill -SYS $PPID'; echo YES)
- * YES    <-- subshell was not killed by SIGSYS
  */
 enum {
 	SPECIAL_INTERACTIVE_SIGS = 0
@@ -2605,43 +2602,51 @@ static void reset_traps_to_defaults(void)
 {
 	/* This function is always called in a child shell
 	 * after fork (not vfork, NOMMU doesn't use this function).
-	 * Child shells are not interactive.
-	 * SIGTTIN/SIGTTOU/SIGTSTP should not have special handling.
-	 * Testcase: (while :; do :; done) + ^Z should background.
-	 * Same goes for SIGTERM, SIGHUP, SIGINT.
 	 */
 	unsigned sig;
 	unsigned mask;
 
+	/* Child shells are not interactive.
+	 * SIGTTIN/SIGTTOU/SIGTSTP should not have special handling.
+	 * Testcase: (while :; do :; done) + ^Z should background.
+	 * Same goes for SIGTERM, SIGHUP, SIGINT.
+	 */
 	if (!G.traps && !(G.non_DFL_mask & SPECIAL_INTERACTIVE_SIGS))
-		return;
+		return; /* already no traps and no SPECIAL_INTERACTIVE_SIGS */
 
-	/* Stupid. It can be done with *single* &= op, but we can't use
+	/* Switching off SPECIAL_INTERACTIVE_SIGS.
-	 * the fact that G.blocked_set is implemented as a bitmask... */
+	 * Stupid. It can be done with *single* &= op, but we can't use
+	 * the fact that G.blocked_set is implemented as a bitmask
+	 * in libc... */
 	mask = (SPECIAL_INTERACTIVE_SIGS >> 1);
 	sig = 1;
 	while (1) {
-		if (mask & 1)
+		if (mask & 1) {
+			/* Careful. Only if no trap or trap is not "" */
+			if (!G.traps || !G.traps[sig] || G.traps[sig][0])
 				sigdelset(&G.blocked_set, sig);
+		}
 		mask >>= 1;
 		if (!mask)
 			break;
 		sig++;
 	}
-
+	/* Our homegrown sig mask is saner to work with :) */
 	G.non_DFL_mask &= ~SPECIAL_INTERACTIVE_SIGS;
+
+	/* Resetting all traps to default except empty ones */
 	mask = G.non_DFL_mask;
 	if (G.traps) for (sig = 0; sig < NSIG; sig++, mask >>= 1) {
-		if (!G.traps[sig])
+		if (!G.traps[sig] || !G.traps[sig][0])
 			continue;
 		free(G.traps[sig]);
 		G.traps[sig] = NULL;
 		/* There is no signal for 0 (EXIT) */
 		if (sig == 0)
 			continue;
-		/* There was a trap handler, we are removing it.
+		/* There was a trap handler, we just removed it.
 		 * But if sig still has non-DFL handling,
-		 * we should not unblock it. */
+		 * we should not unblock the sig. */
 		if (mask & 1)
 			continue;
 		sigdelset(&G.blocked_set, sig);

shell/hush_test/hush-trap/subshell.right

@@ -0,0 +1,6 @@
+Ok
+Ok
+Ok
+Ok
+TERM
+Done

shell/hush_test/hush-trap/subshell.tests

@@ -0,0 +1,20 @@
+# Non-empty traps should be reset in subshell
+
+# HUP is special in interactive shells
+trap '' HUP
+# QUIT is always special
+trap '' QUIT
+# SYS is not special
+trap '' SYS
+# WINCH is harmless
+trap 'bad: caught WINCH' WINCH
+# With TERM we'll check whether it is reset
+trap 'bad: caught TERM'  TERM
+
+# using bash, becuase we don't have $PPID (yet)
+(bash -c 'kill -HUP   $PPID'; echo Ok)
+(bash -c 'kill -QUIT  $PPID'; echo Ok)
+(bash -c 'kill -SYS   $PPID'; echo Ok)
+(bash -c 'kill -WINCH $PPID'; echo Ok)
+(bash -c 'kill -TERM  $PPID'; echo Bad: TERM is not reset)
+echo Done