[git commit] hush: fix corner cases with exec in empty expansions

Denys Vlasenko vda.linux at googlemail.com
Thu Apr 5 12:41:21 UTC 2018


commit: https://git.busybox.net/busybox/commit/?id=41d8f1081378ec79586d59e7d2a31380b6f95577
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

Cases like these:

var=val exec >redir

var=val `` >redir

function                                             old     new   delta
run_pipe                                            1701    1723     +22
redirect_and_varexp_helper                            56      55      -1
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/1 up/down: 22/-1)              Total: 21 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 shell/ash_test/ash-redir/redir_exec1.right   |  2 ++
 shell/ash_test/ash-redir/redir_exec1.tests   |  2 ++
 shell/hush.c                                 | 43 +++++++++++++++++-----------
 shell/hush_test/hush-redir/redir_exec1.right |  3 ++
 shell/hush_test/hush-redir/redir_exec1.tests |  2 ++
 5 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/shell/ash_test/ash-redir/redir_exec1.right b/shell/ash_test/ash-redir/redir_exec1.right
new file mode 100644
index 000000000..d4393d10c
--- /dev/null
+++ b/shell/ash_test/ash-redir/redir_exec1.right
@@ -0,0 +1,2 @@
+redir_exec1.tests: line 1: can't create /cant/be/created: nonexistent directory
+First
diff --git a/shell/ash_test/ash-redir/redir_exec1.tests b/shell/ash_test/ash-redir/redir_exec1.tests
new file mode 100755
index 000000000..290e1cb39
--- /dev/null
+++ b/shell/ash_test/ash-redir/redir_exec1.tests
@@ -0,0 +1,2 @@
+v=`echo First >&2` exec >/cant/be/created
+echo One:$?
diff --git a/shell/hush.c b/shell/hush.c
index 24ae237d7..43702360a 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -8200,19 +8200,21 @@ static int redirect_and_varexp_helper(
 		struct squirrel **sqp,
 		char **argv_expanded)
 {
+	/* Assignments occur before redirects. Try:
+	 * a=`sleep 1` sleep 2 3>/qwe/rty
+	 */
+
+	char **new_env = expand_assignments(command->argv, command->assignment_cnt);
+	dump_cmd_in_x_mode(new_env);
+	dump_cmd_in_x_mode(argv_expanded);
+	/* this takes ownership of new_env[i] elements, and frees new_env: */
+	set_vars_and_save_old(new_env);
+
 	/* setup_redirects acts on file descriptors, not FILEs.
 	 * This is perfect for work that comes after exec().
 	 * Is it really safe for inline use?  Experimentally,
 	 * things seem to work. */
-	int rcode = setup_redirects(command, sqp);
-	if (rcode == 0) {
-		char **new_env = expand_assignments(command->argv, command->assignment_cnt);
-		dump_cmd_in_x_mode(new_env);
-		dump_cmd_in_x_mode(argv_expanded);
-		/* this takes ownership of new_env[i] elements, and frees new_env: */
-		set_vars_and_save_old(new_env);
-	}
-	return rcode;
+	return setup_redirects(command, sqp);
 }
 static NOINLINE int run_pipe(struct pipe *pi)
 {
@@ -8315,6 +8317,7 @@ static NOINLINE int run_pipe(struct pipe *pi)
 			 * Ensure redirects take effect (that is, create files).
 			 * Try "a=t >file"
 			 */
+ only_assignments:
 			G.expand_exitcode = 0;
 
 			rcode = setup_redirects(command, &squirrel);
@@ -8359,12 +8362,10 @@ static NOINLINE int run_pipe(struct pipe *pi)
 #endif
 			argv_expanded = expand_strvec_to_strvec(argv + command->assignment_cnt);
 
-		/* if someone gives us an empty string: `cmd with empty output` */
-//TODO: what about: var=EXPR `` >FILE ? will var be set? Will FILE be created?
+		/* If someone gives us an empty string: `cmd with empty output` */
 		if (!argv_expanded[0]) {
 			free(argv_expanded);
-			debug_leave();
-			return G.last_exitcode;
+			goto only_assignments;
 		}
 
 		old_vars = NULL;
@@ -8378,9 +8379,17 @@ static NOINLINE int run_pipe(struct pipe *pi)
 		if (x || funcp) {
 			if (x && x->b_function == builtin_exec && argv_expanded[1] == NULL) {
 				debug_printf("exec with redirects only\n");
-				rcode = setup_redirects(command, NULL);
-//TODO: what about: var=EXPR exec >FILE ? will var be set?
+				/*
+				 * Variable assignments are executed, but then "forgotten":
+				 *  a=`sleep 1;echo A` exec 3>&-; echo $a
+				 * sleeps, but prints nothing.
+				 */
+				enter_var_nest_level();
+				G.shadowed_vars_pp = &old_vars;
+				rcode = redirect_and_varexp_helper(command, /*squirrel:*/ NULL, argv_expanded);
+				G.shadowed_vars_pp = sv_shadowed;
 				/* rcode=1 can be if redir file can't be opened */
+
 				goto clean_up_and_ret1;
 			}
 
@@ -8452,9 +8461,10 @@ static NOINLINE int run_pipe(struct pipe *pi)
 		} else
 			goto must_fork;
 
+		restore_redirects(squirrel);
+ clean_up_and_ret1:
 		leave_var_nest_level();
 		add_vars(old_vars);
-		restore_redirects(squirrel);
 
 		/*
 		 * Try "usleep 99999999" + ^C + "echo $?"
@@ -8474,7 +8484,6 @@ static NOINLINE int run_pipe(struct pipe *pi)
 			if (sigismember(&G.pending_set, SIGINT))
 				rcode = 128 + SIGINT;
 		}
- clean_up_and_ret1:
 		free(argv_expanded);
 		IF_HAS_KEYWORDS(if (pi->pi_inverted) rcode = !rcode;)
 		debug_leave();
diff --git a/shell/hush_test/hush-redir/redir_exec1.right b/shell/hush_test/hush-redir/redir_exec1.right
new file mode 100644
index 000000000..6ff8fc832
--- /dev/null
+++ b/shell/hush_test/hush-redir/redir_exec1.right
@@ -0,0 +1,3 @@
+First
+hush: can't open '/cant/be/created': No such file or directory
+One:1
diff --git a/shell/hush_test/hush-redir/redir_exec1.tests b/shell/hush_test/hush-redir/redir_exec1.tests
new file mode 100755
index 000000000..290e1cb39
--- /dev/null
+++ b/shell/hush_test/hush-redir/redir_exec1.tests
@@ -0,0 +1,2 @@
+v=`echo First >&2` exec >/cant/be/created
+echo One:$?


More information about the busybox-cvs mailing list