Date: Sun, 12 Apr 2009 23:33:14 +0200 From: Jilles Tjoelker <jilles@stack.nl> To: bug-followup@FreeBSD.org, ed@freebsd.org, olli@lurza.secnetix.de, freebsd-hackers@freebsd.org Subject: Re: bin/113860: sh(1): shell is still running when using `sh -c' Message-ID: <20090412213314.GA57862@stack.nl> In-Reply-To: <20090403213905.GA21297@stack.nl> References: <20090403213905.GA21297@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--9amGYk9869ThD9tj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Apr 03, 2009 at 11:39:05PM +0200, Jilles Tjoelker wrote: > I think this can be improved. I have a patch now; it does not handle all cases but the effect on the code is minimal. I decided to go with the readahead but only do it for the first line. To avoid problems with traps not being executed, http://www.stack.nl/~jilles/unix/sh-forkiftrapped.patch is needed. This fixes a bug in EV_EXIT handling, which would be triggered more often with the change to -c. The patch is also needed for bin/74404. Example: sh -c '(trap "echo trapped" EXIT; sleep 3)' http://www.stack.nl/~jilles/unix/sh-minusc-exec.patch the change itself -- Jilles Tjoelker --9amGYk9869ThD9tj Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="sh-forkiftrapped.patch" Don't skip forking for an external command if any traps are active. Example: sh -c '(trap "echo trapped" EXIT; sleep 3)' now correctly prints "trapped". With this check, it is no longer necessary to check for -T explicitly in that case. diff --git a/eval.c b/eval.c --- a/eval.c +++ b/eval.c @@ -730,7 +730,7 @@ evalcommand(union node *cmd, int flags, /* Fork off a child process if necessary. */ if (cmd->ncmd.backgnd || (cmdentry.cmdtype == CMDNORMAL - && ((flags & EV_EXIT) == 0 || Tflag)) + && ((flags & EV_EXIT) == 0 || have_traps())) || ((flags & EV_BACKCMD) != 0 && (cmdentry.cmdtype != CMDBUILTIN || cmdentry.u.index == CDCMD diff --git a/trap.c b/trap.c --- a/trap.c +++ b/trap.c @@ -222,6 +222,21 @@ clear_traps(void) /* + * Check if we have any traps enabled. + */ +int +have_traps(void) +{ + char *volatile *tp; + + for (tp = trap ; tp <= &trap[NSIG - 1] ; tp++) { + if (*tp && **tp) /* trap not NULL or SIG_IGN */ + return 1; + } + return 0; +} + +/* * Set the signal handler for the specified signal. The routine figures * out what it should be set to. */ diff --git a/trap.h b/trap.h --- a/trap.h +++ b/trap.h @@ -39,6 +39,7 @@ extern volatile sig_atomic_t gotwinch; int trapcmd(int, char **); void clear_traps(void); +int have_traps(void); void setsignal(int); void ignoresig(int); void onsig(int); --9amGYk9869ThD9tj Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="sh-minusc-exec.patch" Avoid leaving unnecessary waiting shells in many forms of sh -c COMMAND. This change only affects strings passed to -c, when the -s option is not used. The approach is to read the first line of commands, then the second, and if it turns out there is no second line execute the first line with EV_EXIT. Otherwise execute the first and second line, then read and execute lines as long as they exist. A compound statement such as introduced by {, if, for or while counts as a single line of commands for this. Note that if the second line contains a syntactical error, the first line will not be executed, different from previously. (pdksh and zsh parse the entire -c string before executing it.) Example: sh -c 'ps lT' No longer shows a shell process waiting for ps to finish. diff --git a/eval.c b/eval.c --- a/eval.c +++ b/eval.c @@ -175,6 +175,44 @@ evalstring(char *s) } +/* + * Like evalstring(), but always exits. This is useful in avoiding shell + * processes just sitting around waiting for exit. + */ + +void +evalstring_exit(char *s) +{ + union node *n, *n2; + struct stackmark smark; + + setstackmark(&smark); + setinputstring(s, 1); + do + n = parsecmd(0); + while (n == NULL); + if (n != NEOF) { + do + n2 = parsecmd(0); + while (n2 == NULL); + if (n2 == NEOF) { + evaltree(n, EV_EXIT); + /*NOTREACHED*/ + } + evaltree(n, 0); + evaltree(n2, 0); + popstackmark(&smark); + while ((n = parsecmd(0)) != NEOF) { + if (n != NULL) + evaltree(n, 0); + popstackmark(&smark); + } + } + popfile(); + popstackmark(&smark); + exitshell(exitstatus); +} + /* * Evaluate a parse tree. The value is left in the global variable diff --git a/eval.h b/eval.h --- a/eval.h +++ b/eval.h @@ -47,6 +47,7 @@ struct backcmd { /* result of evalbackc int evalcmd(int, char **); void evalstring(char *); +void evalstring_exit(char *); union node; /* BLETCH for ansi C */ void evaltree(union node *, int); void evalbackcmd(union node *, struct backcmd *); diff --git a/main.c b/main.c --- a/main.c +++ b/main.c @@ -178,7 +178,10 @@ state2: state3: state = 4; if (minusc) { - evalstring(minusc); + if (sflag) + evalstring(minusc); + else + evalstring_exit(minusc); } if (sflag || minusc == NULL) { state4: /* XXX ??? - why isn't this before the "if" statement */ --9amGYk9869ThD9tj--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090412213314.GA57862>