summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDenys Vlasenko2010-05-18 16:13:56 +0200
committerDenys Vlasenko2010-05-18 16:13:56 +0200
commit42c4b2e3b535314ae8a7b65c3223afb26872d5a2 (patch)
tree8c0d1b5eef44dc9dbce6788fe41db1af22bbf470
parentc7f95d23f6bc7e17a3b79decf83eb362b389e53a (diff)
downloadbusybox-42c4b2e3b535314ae8a7b65c3223afb26872d5a2.zip
busybox-42c4b2e3b535314ae8a7b65c3223afb26872d5a2.tar.gz
ash: fix var_leak.tests so that it actually catches the NOFORK bug
+ document the bug better Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
-rw-r--r--shell/ash.c22
-rw-r--r--shell/ash_test/ash-vars/var_leak.right1
-rwxr-xr-xshell/ash_test/ash-vars/var_leak.tests7
3 files changed, 21 insertions, 9 deletions
diff --git a/shell/ash.c b/shell/ash.c
index d082333..83886c6 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -9179,12 +9179,14 @@ evalcommand(union node *cmd, int flags)
/* Execute the command. */
switch (cmdentry.cmdtype) {
- default:
+ default: {
#if ENABLE_FEATURE_SH_NOFORK
-/* Hmmm... shouldn't it happen somewhere in forkshell() instead?
- * Why "fork off a child process if necessary" doesn't apply to NOFORK? */
- {
+/* (1) BUG: if variables are set, we need to fork, or save/restore them
+ * around run_nofork_applet() call.
+ * (2) Should this check also be done in forkshell()?
+ * (perhaps it should, so that "VAR=VAL nofork" at least avoids exec...)
+ */
/* find_command() encodes applet_no as (-2 - applet_no) */
int applet_no = (- cmdentry.u.index - 2);
if (applet_no >= 0 && APPLET_IS_NOFORK(applet_no)) {
@@ -9193,10 +9195,13 @@ evalcommand(union node *cmd, int flags)
exitstatus = run_nofork_applet(applet_no, argv);
break;
}
- }
#endif
- /* Fork off a child process if necessary. */
+ /* Can we avoid forking off? For example, very last command
+ * in a script or a subshell does not need forking,
+ * we can just exec it.
+ */
if (!(flags & EV_EXIT) || may_have_traps) {
+ /* No, forking off a child is necessary */
INT_OFF;
jp = makejob(/*cmd,*/ 1);
if (forkshell(jp, cmd, FORK_FG) != 0) {
@@ -9213,7 +9218,7 @@ evalcommand(union node *cmd, int flags)
listsetvar(varlist.list, VEXPORT|VSTACK);
shellexec(argv, path, cmdentry.u.index);
/* NOTREACHED */
-
+ } /* default */
case CMDBUILTIN:
cmdenviron = varlist.list;
if (cmdenviron) {
@@ -9258,7 +9263,8 @@ evalcommand(union node *cmd, int flags)
if (evalfun(cmdentry.u.func, argc, argv, flags))
goto raise;
break;
- }
+
+ } /* switch */
out:
popredir(/*drop:*/ cmd_is_exec, /*restore:*/ cmd_is_exec);
diff --git a/shell/ash_test/ash-vars/var_leak.right b/shell/ash_test/ash-vars/var_leak.right
index be78112..01a5e32 100644
--- a/shell/ash_test/ash-vars/var_leak.right
+++ b/shell/ash_test/ash-vars/var_leak.right
@@ -1,3 +1,4 @@
should be empty: ''
+should be empty: ''
should be not empty: 'val2'
should be not empty: 'val3'
diff --git a/shell/ash_test/ash-vars/var_leak.tests b/shell/ash_test/ash-vars/var_leak.tests
index 0320592..5242e24 100755
--- a/shell/ash_test/ash-vars/var_leak.tests
+++ b/shell/ash_test/ash-vars/var_leak.tests
@@ -1,6 +1,11 @@
-# true is a regular builtin, varibale should not leak out of it
+# cat is an external program, variable should not leak out of it.
# this currently fails with CONFIG_FEATURE_SH_NOFORK=y
VAR=''
+VAR=val0 cat /dev/null
+echo "should be empty: '$VAR'"
+
+# true is a regular builtin, variable should not leak out of it.
+VAR=''
VAR=val1 true
echo "should be empty: '$VAR'"