[git commit] hush: fix "true | func_with_return" not allowing return.
Denys Vlasenko
vda.linux at googlemail.com
Mon Jul 31 16:02:28 UTC 2017
commit: https://git.busybox.net/busybox/commit/?id=5b3d2eb327ce7e1c55c6c94bc70782f733d59990
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master
function old new delta
pseudo_exec_argv 305 312 +7
Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
.../ash-misc/{func6.right => func_return1.right} | 0
.../ash-misc/{func6.tests => func_return1.tests} | 0
.../ash-misc/func_return2.right} | 0
shell/ash_test/ash-misc/func_return2.tests | 6 +++
shell/hush.c | 44 ++++++++++++++++++----
.../hush-misc/func_return1.right} | 0
.../hush-misc/{func6.tests => func_return1.tests} | 0
.../hush-misc/func_return2.right} | 0
shell/hush_test/hush-misc/func_return2.tests | 6 +++
9 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/shell/ash_test/ash-misc/func6.right b/shell/ash_test/ash-misc/func_return1.right
similarity index 100%
copy from shell/ash_test/ash-misc/func6.right
copy to 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
similarity index 100%
rename from shell/ash_test/ash-misc/func6.tests
rename to 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
similarity index 100%
rename from shell/hush_test/hush-misc/func6.right
rename to 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/ash_test/ash-misc/func6.right b/shell/hush_test/hush-misc/func_return1.right
similarity index 100%
copy from shell/ash_test/ash-misc/func6.right
copy to shell/hush_test/hush-misc/func_return1.right
diff --git a/shell/hush_test/hush-misc/func6.tests b/shell/hush_test/hush-misc/func_return1.tests
similarity index 100%
rename from shell/hush_test/hush-misc/func6.tests
rename to shell/hush_test/hush-misc/func_return1.tests
diff --git a/shell/ash_test/ash-misc/func6.right b/shell/hush_test/hush-misc/func_return2.right
similarity index 100%
rename from shell/ash_test/ash-misc/func6.right
rename to shell/hush_test/hush-misc/func_return2.right
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:$?
More information about the busybox-cvs
mailing list