From owner-freebsd-hackers@FreeBSD.ORG Sun Apr 12 21:33:34 2009 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1C0671065753; Sun, 12 Apr 2009 21:33:34 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (unknown [IPv6:2001:610:1108:5000::149]) by mx1.freebsd.org (Postfix) with ESMTP id 9F31D8FC2F; Sun, 12 Apr 2009 21:33:30 +0000 (UTC) (envelope-from jilles@stack.nl) Received: by mx1.stack.nl (Postfix, from userid 65534) id 7E9403FA87; Sun, 12 Apr 2009 23:33:29 +0200 (CEST) X-Spam-DCC: : X-Spam-Checker-Version: SpamAssassin 3.2.3 (2007-08-08) on meestal-mk5.stack.nl X-Spam-Level: X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,J_CHICKENPOX_73, J_CHICKENPOX_81,NO_RELAYS autolearn=no version=3.2.3 X-Spam-Relay-Country: Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 1A0A33F82C; Sun, 12 Apr 2009 23:33:27 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id 65851228A3; Sun, 12 Apr 2009 23:33:14 +0200 (CEST) Date: Sun, 12 Apr 2009 23:33:14 +0200 From: Jilles Tjoelker To: bug-followup@FreeBSD.org, ed@freebsd.org, olli@lurza.secnetix.de, freebsd-hackers@freebsd.org Message-ID: <20090412213314.GA57862@stack.nl> References: <20090403213905.GA21297@stack.nl> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="9amGYk9869ThD9tj" Content-Disposition: inline In-Reply-To: <20090403213905.GA21297@stack.nl> User-Agent: Mutt/1.5.18 (2008-05-17) Cc: Subject: Re: bin/113860: sh(1): shell is still running when using `sh -c' X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 12 Apr 2009 21:33:36 -0000 --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--