hush: fix "true | func_with_return" not allowing return.

function                                             old     new   delta
pseudo_exec_argv                                     305     312      +7

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

9 files changed, 52 insertions, 8 deletions

diff --git a/shell/ash_test/ash-misc/func6.right b/shell/ash_test/ash-misc/func_return1.right
index 0ebd8e5..0ebd8e5 100644
--- a/shell/ash_test/ash-misc/func6.right
+++ b/shell/ash_test/ash-misc/func_return1.right
diff --git a/shell/ash_test/ash-misc/func6.tests b/shell/ash_test/ash-misc/func_return1.tests
index 029c3e8..029c3e8 100755
--- a/shell/ash_test/ash-misc/func6.tests
+++ b/shell/ash_test/ash-misc/func_return1.tests
diff --git a/shell/hush_test/hush-misc/func6.right b/shell/ash_test/ash-misc/func_return2.right
index 0ebd8e5..0ebd8e5 100644
--- a/shell/hush_test/hush-misc/func6.right
+++ b/shell/ash_test/ash-misc/func_return2.right
diff --git a/shell/ash_test/ash-misc/func_return2.tests b/shell/ash_test/ash-misc/func_return2.tests
new file mode 100755
index 0000000..a049dd1
--- /dev/null
+++ b/shell/ash_test/ash-misc/func_return2.tests
@@ -0,0 +1,6 @@
+f1() { return 2; }
+f1
+echo Two:$?
+false
+true | f1
+echo Two:$?
diff --git a/shell/hush.c b/shell/hush.c
index 2fba637..5c8e007 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -6804,7 +6804,7 @@ static void restore_redirects(struct squirrel *sq)
 }
 
 #if ENABLE_FEATURE_SH_STANDALONE && BB_MMU
-static void close_saved_fds_and_FILE_list(void)
+static void close_saved_fds_and_FILE_fds(void)
 {
 	if (G_interactive_fd)
 		close(G_interactive_fd);
@@ -7090,13 +7090,35 @@ static void exec_function(char ***to_free,
 	argv[0] = G.global_argv[0];
 	G.global_argv = argv;
 	G.global_argc = n = 1 + string_array_len(argv + 1);
-//?	close_saved_fds_and_FILE_list();
+
+// Example when we are here: "cmd | func"
+// func will run with saved-redirect fds open.
+// $ f() { echo /proc/self/fd/*; }
+// $ true | f
+// /proc/self/fd/0 /proc/self/fd/1 /proc/self/fd/2 /proc/self/fd/255 /proc/self/fd/3
+// stdio^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ G_interactive_fd^ DIR fd for glob
+// Same in script:
+// $ . ./SCRIPT
+// /proc/self/fd/0 /proc/self/fd/1 /proc/self/fd/2 /proc/self/fd/255 /proc/self/fd/3 /proc/self/fd/4
+// stdio^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ G_interactive_fd^ opened ./SCRIPT DIR fd for glob
+// They are CLOEXEC so external programs won't see them, but
+// for "more correctness" we might want to close those extra fds here:
+//?	close_saved_fds_and_FILE_fds();
+
+	/* "we are in function, ok to use return" */
+	G_flag_return_in_progress = -1;
+	IF_HUSH_LOCAL(G.func_nest_level++;)
+
+	G_flag_return_in_progress = -1;
 	/* On MMU, funcp->body is always non-NULL */
 	n = run_list(funcp->body);
 	fflush_all();
 	_exit(n);
 # else
-//?	close_saved_fds_and_FILE_list();
+//?	close_saved_fds_and_FILE_fds();
+
+//TODO: check whether "true | func_with_return" works
+
 	re_execute_shell(to_free,
 			funcp->body_as_string,
 			G.global_argv[0],
@@ -7116,9 +7138,7 @@ static int run_function(const struct function *funcp, char **argv)
 	/* "we are in function, ok to use return" */
 	sv_flg = G_flag_return_in_progress;
 	G_flag_return_in_progress = -1;
-# if ENABLE_HUSH_LOCAL
-	G.func_nest_level++;
-# endif
+	IF_HUSH_LOCAL(G.func_nest_level++;)
 
 	/* On MMU, funcp->body is always non-NULL */
 # if !BB_MMU
@@ -7182,7 +7202,7 @@ static void exec_builtin(char ***to_free,
 #if BB_MMU
 	int rcode;
 	fflush_all();
-//?	close_saved_fds_and_FILE_list();
+//?	close_saved_fds_and_FILE_fds();
 	rcode = x->b_function(argv);
 	fflush_all();
 	_exit(rcode);
@@ -7342,7 +7362,7 @@ static NOINLINE void pseudo_exec_argv(nommu_save_t *nommu_save,
 				 * Testcase: interactive "ls -l /proc/self/fd"
 				 * should not show tty fd open.
 				 */
-				close_saved_fds_and_FILE_list();
+				close_saved_fds_and_FILE_fds();
 //FIXME: should also close saved redir fds
 				debug_printf_exec("running applet '%s'\n", argv[0]);
 				run_applet_no_and_exit(a, argv[0], argv);
@@ -9253,6 +9273,14 @@ static int FAST_FUNC builtin_exec(char **argv)
 	if (G_saved_tty_pgrp && getpid() == G.root_pid)
 		tcsetpgrp(G_interactive_fd, G_saved_tty_pgrp);
 
+	/* Saved-redirect fds, script fds and G_interactive_fd are still
+	 * open here. However, they are all CLOEXEC, and execv below
+	 * closes them. Try interactive "exec ls -l /proc/self/fd",
+	 * it should show no extra open fds in the "ls" process.
+	 * If we'd try to run builtins/NOEXECs, this would need improving.
+	 */
+	//close_saved_fds_and_FILE_fds();
+
 	/* TODO: if exec fails, bash does NOT exit! We do.
 	 * We'll need to undo trap cleanup (it's inside execvp_or_die)
 	 * and tcsetpgrp, and this is inherently racy.
diff --git a/shell/hush_test/hush-misc/func_return1.right b/shell/hush_test/hush-misc/func_return1.right
new file mode 100644
index 0000000..0ebd8e5
--- /dev/null
+++ b/shell/hush_test/hush-misc/func_return1.right
@@ -0,0 +1,2 @@
+Two:2
+Two:2
diff --git a/shell/hush_test/hush-misc/func6.tests b/shell/hush_test/hush-misc/func_return1.tests
index 029c3e8..029c3e8 100755
--- a/shell/hush_test/hush-misc/func6.tests
+++ b/shell/hush_test/hush-misc/func_return1.tests
diff --git a/shell/hush_test/hush-misc/func_return2.right b/shell/hush_test/hush-misc/func_return2.right
new file mode 100644
index 0000000..0ebd8e5
--- /dev/null
+++ b/shell/hush_test/hush-misc/func_return2.right
@@ -0,0 +1,2 @@
+Two:2
+Two:2
diff --git a/shell/hush_test/hush-misc/func_return2.tests b/shell/hush_test/hush-misc/func_return2.tests
new file mode 100755
index 0000000..a049dd1
--- /dev/null
+++ b/shell/hush_test/hush-misc/func_return2.tests
@@ -0,0 +1,6 @@
+f1() { return 2; }
+f1
+echo Two:$?
+false
+true | f1
+echo Two:$?