DASH Shell discussions
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: dash@vger.kernel.org
Cc: Jilles Tjoelker <jilles@stack.nl>,
	Drake Wilson <drake@begriffli.ch>, Reuben Thomas <rrt@sc3d.org>
Subject: [PATCH 5/4] [EVAL] Remove unused EV_BACKCMD flag
Date: Sun, 10 Apr 2011 02:38:37 -0500	[thread overview]
Message-ID: <20110410073837.GE17649@elie> (raw)
In-Reply-To: <20110410071734.GA16736@elie>

The original ash defered forking commands in backquotes so builtins
could be run in the same context as the shell.  This behavior was
controlled using the EV_BACKCMD to evaltree.

Unfortunately, as Matthias Scheler noticed in 1999 (NetBSD PR/7814),
the result was counterintuitive; for example, echo "`cd /`" would
change the cwd.  So ash 0.3.5 left out that optimization.  The
EV_BACKCMD codepath stayed around, unused.

Some time between ash 0.3.5-11 and ash 0.3.8-37, Debian ash omitted
the EV_BACKCMD pathway by guarding it with #ifdef notyet.  In dash
0.5.1 and later, the commented code is no more.  Let's finish the job
and remove the last vestiges.  If someone wants to work on omitting
the fork in backcmd, the remaining hints are not going to be very
helpful, anyway.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Thanks for reading.  Thoughts?

The new sizes in the cover letter are overestimated by 16 bytes.
Sorry about that.

 src/eval.c |   63 ++++++++++++++++++-----------------------------------------
 src/eval.h |    1 -
 2 files changed, 19 insertions(+), 45 deletions(-)

diff --git a/src/eval.c b/src/eval.c
index 77a9d00..120e186 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -592,6 +592,9 @@ evalpipe(union node *n, int flags)
 void
 evalbackcmd(union node *n, struct backcmd *result)
 {
+	int pip[2];
+	struct job *jp;
+
 	result->fd = -1;
 	result->buf = NULL;
 	result->nleft = 0;
@@ -600,52 +603,24 @@ evalbackcmd(union node *n, struct backcmd *result)
 		goto out;
 	}
 
-#ifdef notyet
-	/*
-	 * For now we disable executing builtins in the same
-	 * context as the shell, because we are not keeping
-	 * enough state to recover from changes that are
-	 * supposed only to affect subshells. eg. echo "`cd /`"
-	 */
-	if (n->type == NCMD) {
-		struct ifsregion saveifs;
-		struct ifsregion *savelastp;
-		struct nodelist *saveargbackq;
-
-		saveifs = ifsfirst;
-		savelastp = ifslastp;
-		saveargbackq = argbackq;
-
-		exitstatus = oexitstatus;
-		evalcommand(n, EV_BACKCMD, result);
-
-		ifsfirst = saveifs;
-		ifslastp = savelastp;
-		argbackq = saveargbackq;
-	} else
-#endif
-	{
-		int pip[2];
-		struct job *jp;
-
-		if (pipe(pip) < 0)
-			sh_error("Pipe call failed");
-		jp = makejob(n, 1);
-		if (forkshell(jp, n, FORK_NOJOB) == 0) {
-			FORCEINTON;
-			close(pip[0]);
-			if (pip[1] != 1) {
-				dup2(pip[1], 1);
-				close(pip[1]);
-			}
-			ifsfree();
-			evaltreenr(n, EV_EXIT);
-			/* NOTREACHED */
+	if (pipe(pip) < 0)
+		sh_error("Pipe call failed");
+	jp = makejob(n, 1);
+	if (forkshell(jp, n, FORK_NOJOB) == 0) {
+		FORCEINTON;
+		close(pip[0]);
+		if (pip[1] != 1) {
+			dup2(pip[1], 1);
+			close(pip[1]);
 		}
-		close(pip[1]);
-		result->fd = pip[0];
-		result->jp = jp;
+		ifsfree();
+		evaltreenr(n, EV_EXIT);
+		/* NOTREACHED */
 	}
+	close(pip[1]);
+	result->fd = pip[0];
+	result->jp = jp;
+
 out:
 	TRACE(("evalbackcmd done: fd=%d buf=0x%x nleft=%d jp=0x%x\n",
 		result->fd, result->buf, result->nleft, result->jp));
diff --git a/src/eval.h b/src/eval.h
index b1b13f5..4e4de30 100644
--- a/src/eval.h
+++ b/src/eval.h
@@ -49,7 +49,6 @@ struct backcmd {		/* result of evalbackcmd */
 /* flags in argument to evaltree */
 #define EV_EXIT 01		/* exit after evaluating tree */
 #define EV_TESTED 02		/* exit status is checked; ignore -e flag */
-#define EV_BACKCMD 04		/* command executing within back quotes */
 
 int evalstring(char *, int);
 union node;	/* BLETCH for ansi C */
-- 
1.7.5.rc0


  parent reply	other threads:[~2011-04-10  7:38 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-10  7:18 [PATCH/RFC dash 0/4] Avoid a fork before running last command given to -c Jonathan Nieder
2011-04-10  7:21 ` [PATCH 1/4] [INPUT] Introduce preadateof predicate to check for end of input Jonathan Nieder
2011-04-10  7:22 ` [PATCH 2/4] [EVAL] Make eval flags public Jonathan Nieder
2011-04-10  7:35 ` [PATCH 3/4] [EVAL] Take advantage of EV_EXIT in evalstring Jonathan Nieder
2011-04-10  7:36 ` [PATCH 4/4] [MAIN] Optimize dash -c "command" to avoid a fork Jonathan Nieder
2011-07-07  3:48   ` Herbert Xu
2011-07-07  4:27     ` Jonathan Nieder
2011-07-07  4:57       ` Herbert Xu
2011-07-07  5:56         ` Herbert Xu
2011-07-07  7:48           ` Jonathan Nieder
2011-07-07  8:22             ` Herbert Xu
2011-07-07  8:37               ` Jonathan Nieder
2011-07-07  8:39                 ` Herbert Xu
2011-04-10  7:38 ` Jonathan Nieder [this message]
2011-07-07  3:56   ` [PATCH 5/4] [EVAL] Remove unused EV_BACKCMD flag Herbert Xu
2011-04-15 13:07 ` [PATCH/RFC dash 0/4] Avoid a fork before running last command given to -c Herbert Xu
2011-04-17 22:13   ` Jilles Tjoelker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110410073837.GE17649@elie \
    --to=jrnieder@gmail.com \
    --cc=dash@vger.kernel.org \
    --cc=drake@begriffli.ch \
    --cc=jilles@stack.nl \
    --cc=rrt@sc3d.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox