Re: [PATCH] eval: Return status in eval functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/06/16 12:13, Herbert Xu wrote:
On Sat, Dec 05, 2015 at 04:40:59PM +0100, Harald van Dijk wrote:

used to print Fail, and needed the same modification in the evalstring
function to make that print OK (included in the attached patch). There
may be other similar bugs lurking.

Not exactly similar but there are a bunch of bugs caused by setting
exitstatus too early (i.e., before $? has been expanded).

This patch should fix those problems plus the one that you fixed.

Nice! It does indeed fix additional problems. It also introduces a few new bugs though (modified from the FreeBSD testsuite):

src/dash -c 'eval "false

"'; echo $?

This should print 1 and used to print 1, but with this patch applied, it prints 0. The parsing here adds a null commands, which shouldn't affect the exit status.

Unfortunately, attempting to fix this by simply setting
status = exitstatus for null commands breaks other things: given

src/dash -c 'false; case x in *) esac'; echo $?

the correct output is 0, dash used to print 0, and your patch doesn't change that, but attempting to fix the earlier problem by setting status = exitstatus for null commands makes this print 1.

Another one modified from the FreeBSD testsuite:

src/dash -xc 'f() {
  trap "return 1" USR1
  kill -USR1 $$
}
f'; echo $?

This should print 1 and used to print 1, but with this patch applied, it prints 0.

@@ -588,10 +588,12 @@ evalpipe(union node *n, int flags)
  		close(pip[1]);
  	}
  	if (n->npipe.backgnd == 0) {
-		exitstatus = waitforjob(jp);
+		status = waitforjob(jp);
  		TRACE(("evalpipe:  job done exit status %d\n", exitstatus));

This line looks like it should be updated to print status rather than exitstatus when tracing is enabled. :)

Cheers,
Harald van Dijk
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux