summary refs log tree commit diff
diff options
context:
space:
mode:
authorHerbert Xu <herbert@gondor.apana.org.au>2014-10-06 10:39:47 +0800
committerHerbert Xu <herbert@gondor.apana.org.au>2014-10-06 10:39:47 +0800
commitda30b4b787695fbf77e5d941ff350a66ca572bcb (patch)
tree42950a7661b842fe4279bf5c28684def3c8c8fd8
parent[EVAL] Do not clobber exitstatus in evalcommand (diff)
downloaddash-da30b4b787695fbf77e5d941ff350a66ca572bcb.tar.gz
dash-da30b4b787695fbf77e5d941ff350a66ca572bcb.zip
[BUILTIN] Exit without arguments in a trap should use status outside traps
POSIX now requires that exit without arguments in a trap should
return the last command status prior to executing traps.  This
patch implements this behaviour.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
-rw-r--r--ChangeLog4
-rw-r--r--src/eval.c5
-rw-r--r--src/eval.h1
-rw-r--r--src/main.c11
-rw-r--r--src/trap.c25
5 files changed, 33 insertions, 13 deletions
diff --git a/ChangeLog b/ChangeLog
index 4dc8fa3..d0ec202 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2014-10-06  Herbert Xu <herbert@gondor.apana.org.au>
+
+	* Exit without arguments in a trap should use status outside traps.
+
 2014-10-03  Herbert Xu <herbert@gondor.apana.org.au>
 
 	* Do not clobber exitstatus in evalcommand.
diff --git a/src/eval.c b/src/eval.c
index 3f4d957..51a900d 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -74,6 +74,7 @@ static int funcline;		/* starting line number of current function, or 0 if not i
 char *commandname;
 int exitstatus;			/* exit status of last command */
 int back_exitstatus;		/* exit status of backquoted command */
+int savestatus = -1;		/* exit status of last command outside traps */
 
 
 #if !defined(__alpha__) || (defined(__GNUC__) && __GNUC__ >= 3)
@@ -114,6 +115,10 @@ INCLUDE "eval.h"
 RESET {
 	evalskip = 0;
 	loopnest = 0;
+	if (savestatus >= 0) {
+		exitstatus = savestatus;
+		savestatus = -1;
+	}
 }
 #endif
 
diff --git a/src/eval.h b/src/eval.h
index dc8acd2..6e62137 100644
--- a/src/eval.h
+++ b/src/eval.h
@@ -37,6 +37,7 @@
 extern char *commandname;	/* currently executing command */
 extern int exitstatus;		/* exit status of last command */
 extern int back_exitstatus;	/* exit status of backquoted command */
+extern int savestatus;		/* exit status of last command outside traps */
 
 
 struct backcmd {		/* result of evalbackcmd */
diff --git a/src/main.c b/src/main.c
index f79ad7d..29a258d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -339,8 +339,15 @@ exitcmd(int argc, char **argv)
 {
 	if (stoppedjobs())
 		return 0;
-	if (argc > 1)
-		exitstatus = number(argv[1]);
+
+	if (argc > 1) {
+		int status = number(argv[1]);
+
+		exitstatus = status;
+		if (savestatus >= 0)
+			savestatus = status;
+	}
+
 	exraise(EXEXIT);
 	/* NOTREACHED */
 }
diff --git a/src/trap.c b/src/trap.c
index e34e0f8..15faeff 100644
--- a/src/trap.c
+++ b/src/trap.c
@@ -314,12 +314,17 @@ void dotrap(void)
 	char *p;
 	char *q;
 	int i;
-	int savestatus;
+	int status, last_status;
 
 	if (!pendingsigs)
 		return;
 
-	savestatus = exitstatus;
+	status = savestatus;
+	last_status = status;
+	if (likely(status < 0)) {
+		status = exitstatus;
+		savestatus = status;
+	}
 	pendingsigs = 0;
 	barrier();
 
@@ -338,8 +343,10 @@ void dotrap(void)
 		if (!p)
 			continue;
 		evalstring(p, 0);
-		exitstatus = savestatus;
+		exitstatus = status;
 	}
+
+	savestatus = last_status;
 }
 
 
@@ -373,18 +380,14 @@ exitshell(void)
 {
 	struct jmploc loc;
 	char *p;
-	volatile int status;
 
 #ifdef HETIO
 	hetio_reset_term();
 #endif
-	status = exitstatus;
-	TRACE(("pid %d, exitshell(%d)\n", getpid(), status));
-	if (setjmp(loc.loc)) {
-		if (exception == EXEXIT)
-			status = exitstatus;
+	savestatus = exitstatus;
+	TRACE(("pid %d, exitshell(%d)\n", getpid(), savestatus));
+	if (setjmp(loc.loc))
 		goto out;
-	}
 	handler = &loc;
 	if ((p = trap[0])) {
 		trap[0] = NULL;
@@ -399,7 +402,7 @@ out:
 	if (likely(!setjmp(loc.loc)))
 		setjobctl(0);
 	flushall();
-	_exit(status);
+	_exit(savestatus);
 	/* NOTREACHED */
 }
 
e an effect? Perhaps >>> they're not supposed to have any effect, perhaps it's a specific >>> combination of empty regions and something else that triggers some bug, >>> and perhaps that combination can no longer occur with your patch. >> >> The latter is my guess, but I haven't had time to investigate it. > > Looking into it again: > > When IFS is set to an empty string, sepc is set to '\0' in varvalue(). > This then causes *quotedp to be set to true, meaning evalvar()'s quoted > variable is turned on. quoted is then passed to recordregion() as the > nulonly parameter. > > ifsp->nulonly has a bigger effect than merely selecting whether to use > $IFS or whether to only split on null bytes: in ifsbreakup(), nulonly > also causes string termination to be suppressed. That's correct: that > special treatment is required to preserve empty fields in "$@" > expansion. But it should *only* be used when $@ is quoted: ifsbreakup() > takes nulonly from the last IFS region, even if it's empty, so having an > additional zero-length region with nulonly enabled causes confusion. > > Passing quoted by value to varvalue() and not attempting to modify it > should therefore, and in my quick testing does, also work to fix the > original $@ bug. You're right. The proper fix to this is to ensure that nulonly is not set in varvalue for $*. It should only be set for $@ when it's inside double quotes. In fact there is another bug while we're playing with $@/$*. When IFS is set to a non-whitespace character such as :, $* outside quotes won't remove empty fields as it should. This patch fixes both problems. Reported-by: Martijn Dekker <martijn@inlv.org> Suggested-by: Harald van Dijk <harald@gigawatt.nl> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> 2018-04-02parser: Allow newlines within parameter substitutionHerbert Xu On Fri, Mar 16, 2018 at 11:27:22AM +0800, Herbert Xu wrote: > On Thu, Mar 15, 2018 at 10:49:15PM +0100, Harald van Dijk wrote: > > > > Okay, it can be trivially modified to something that does work in other > > shells (even if it were actually executed), but gets rejected at parse time > > by dash: > > > > if false; then > > : ${$+ > > } > > fi > > That's just a bug in dash's parser with ${} in general, because > it bombs out without the if clause too: > > : ${$+ > } This patch fixes the parsing of newlines with parameter substitution. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> 2018-04-02expand: Fix bugs with words connected to the right of $@Herbert Xu On Sun, Mar 04, 2018 at 12:44:59PM +0100, Harald van Dijk wrote: > > command: set -- a ""; space=" "; printf "<%s>" "$@"$space > bash: <a><> > dash 0.5.8: <a>< > > dash 0.5.9.1: <a>< > > dash patched: <a><> This is actually composed of two bugs. First of all our tracking of quotemark is wrong so anything after "$@" becomes quoted. Once we fix that then the problem is that the first space character after "$@" is not recognised as an IFS. This patch fixes both. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> 2018-03-25Revert "[BUILTIN] Remove unnecessary restoration of format string in printf"Herbert Xu This reverts commit 7bb413255368e94395237d789f522891093c5774. The commit breaks printf with more than argument. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> 2018-03-22parser: Fix backquote support in here-document EOF markHerbert Xu