dash.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Harald van Dijk <harald@gigawatt.nl>
To: Herbert Xu <herbert@gondor.apana.org.au>, dash@vger.kernel.org
Cc: Gioele Barabucci <gioele@svario.it>
Subject: Re: [PATCH] eval: Return status in eval functions
Date: Mon, 6 Jun 2016 23:14:40 +0200	[thread overview]
Message-ID: <5755E7C0.8010309@gigawatt.nl> (raw)
In-Reply-To: <20160606101337.GA21441@gondor.apana.org.au>

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

  reply	other threads:[~2016-06-06 21:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-17  2:18 Sourcing an empty files does not reset exit status Gioele Barabucci
2015-12-05 15:40 ` Harald van Dijk
2016-06-06 10:13   ` [PATCH] eval: Return status in eval functions Herbert Xu
2016-06-06 21:14     ` Harald van Dijk [this message]
2016-06-07  8:47       ` [PATCH v2] " Herbert Xu

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=5755E7C0.8010309@gigawatt.nl \
    --to=harald@gigawatt.nl \
    --cc=dash@vger.kernel.org \
    --cc=gioele@svario.it \
    --cc=herbert@gondor.apana.org.au \
    --subject='Re: [PATCH] eval: Return status in eval functions' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).