bin/113860: sh(1): shell is still running when using `sh -c'

[Jilles Tjoelker]

The following reply was made to PR bin/113860; it has been noted by GNATS.

From: Jilles Tjoelker <jilles at stack.nl>
To: bug-followup at FreeBSD.org, ed at freebsd.org, olli at lurza.secnetix.de,
	freebsd-hackers at freebsd.org
Cc:  
Subject: Re: bin/113860: sh(1): shell is still running when using `sh -c'
Date: Sun, 12 Apr 2009 23:33:14 +0200

 --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--