debian patches to exit with code 127 for nonexistent/directory scripts

debian patches to exit with code 127 for nonexistent/directory scripts

[Jilles Tjoelker]

Debian's dash package has some local changes which cause an exit with
code 127, as required by POSIX, if a script (passed with dash
<filename>) cannot be opened or cannot be read because it is a
directory.

Unfortunately, these patches also affect the . builtin (if the pathname
contains a slash) and use EXEXIT, which means such errors always cause
the shell to exit, even in interactive mode or if the builtin's
specialness has been disabled using command.

% dash
$ . ./nonexistent
.: 1: Can't open ./nonexistent
zsh: exit 127   dash
%

% dash -c 'command . ./nonexistent; echo continued'

Note: Do not compare this with bash. Bash deliberately does not follow
POSIX XCU 2.8.1 Consequences of Shell Errors if not in POSIX mode, and
even in POSIX mode trying to source a nonexistent dot script (without
slash in the pathname) fails to abort the shell.

Note 2: POSIX seems unclear about what 'command .' should do, but is
very clear that failure to find/read a dot script shall not cause an
interactive shell to exit.

-- 
Jilles Tjoelker

Re: debian patches to exit with code 127 for nonexistent/directory scripts

[Herbert Xu]

Jilles Tjoelker <jilles@stack.nl> wrote:
> Debian's dash package has some local changes which cause an exit with
> code 127, as required by POSIX, if a script (passed with dash
> <filename>) cannot be opened or cannot be read because it is a
> directory.

Please report this through Debian's bug tracking system.

There is nothing that I can do about this.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: debian patches to exit with code 127 for nonexistent/directory scripts

[Cristian Ionescu-Idbohrn]

On Tue, 8 Jun 2010, Herbert Xu wrote:

> Jilles Tjoelker <jilles@stack.nl> wrote:
> > Debian's dash package has some local changes which cause an exit
> > with code 127, as required by POSIX, if a script (passed with dash
> > <filename>) cannot be opened or cannot be read because it is a
> > directory.

My interpretation of the above is that debian's patched dash is POSIX
compliant WRT the named exit code.

> Please report this through Debian's bug tracking system.

Why?

> There is nothing that I can do about this.

Upstream dash is _not_ POSIX compliant WRT the named exit code, seems
to be the meaning of the sentence.  And you can certainly _do_
something about it.

On the other hand, I may have got it all wrong (because of some
language barrier), in which case please accept my apologies.


Cheers,

-- 
Cristian

Re: debian patches to exit with code 127 for nonexistent/directory scripts

[Herbert Xu]

Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn@axis.com> wrote:
> On Tue, 8 Jun 2010, Herbert Xu wrote:
> 
>> Jilles Tjoelker <jilles@stack.nl> wrote:
>> > Debian's dash package has some local changes which cause an exit
>> > with code 127, as required by POSIX, if a script (passed with dash
>> > <filename>) cannot be opened or cannot be read because it is a
>> > directory.
> 
> My interpretation of the above is that debian's patched dash is POSIX
> compliant WRT the named exit code.

Well if dash is broken then you should submit a patch.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] [INPUT] exit 127 if command_file is given but doesn't exist

[Gerrit Pape]

This commit makes dash exit with return code 127 instead of 2 if
started as non-interactive shell with a non-existent command_file
specified as argument (or a directory), as documented in
 http://www.opengroup.org/onlinepubs/009695399/utilities/sh.html#tag_04_128_14

The wrong exit code was reported by Clint Adams and Jari Aalto through
 http://bugs.debian.org/548743
 http://bugs.debian.org/548687

Signed-off-by: Gerrit Pape <pape@smarden.org>
---

On Sat, Jun 05, 2010 at 06:06:51PM +0200, Jilles Tjoelker wrote:
> Debian's dash package has some local changes which cause an exit with
> code 127, as required by POSIX, if a script (passed with dash
> <filename>) cannot be opened or cannot be read because it is a
> directory.
>
> Unfortunately, these patches also affect the . builtin (if the
> pathname
> contains a slash) and use EXEXIT, which means such errors always cause
> the shell to exit, even in interactive mode or if the builtin's
> specialness has been disabled using command.

Hi Jilles, thanks for the hint, I didn't notice.  I hope this patch does
better, it replaces these two
 http://article.gmane.org/gmane.comp.shells.dash/198
 http://article.gmane.org/gmane.comp.shells.dash/199

Regards, Gerrit.

 src/error.h |    1 +
 src/input.c |    7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/error.h b/src/error.h
index 3162e15..c75cb2c 100644
--- a/src/error.h
+++ b/src/error.h
@@ -69,6 +69,7 @@ extern int exception;
 #define EXSHELLPROC 2	/* execute a shell procedure */
 #define EXEXEC 3	/* command execution failed */
 #define EXEXIT 4	/* exit the shell */
+#define EXIFILE 5	/* inputfile cannot be opened or is a directory */
 
 
 /*
diff --git a/src/input.c b/src/input.c
index e57ad76..a43a7b9 100644
--- a/src/input.c
+++ b/src/input.c
@@ -54,6 +54,7 @@
 #include "parser.h"
 #include "main.h"
 #include "var.h"
+#include "eval.h"
 #ifndef SMALL
 #include "myhistedit.h"
 #endif
@@ -405,11 +406,13 @@ setinputfile(const char *fname, int flags)
 	int fd;
 
 	INTOFF;
-	if ((fd = open(fname, O_RDONLY)) < 0) {
+	if ((fd = open(fname, O_RDWR)) < 0) {
 		if (flags & INPUT_NOFILE_OK)
 			goto out;
-		sh_error("Can't open %s", fname);
+		exitstatus = 127;
+		exerror(EXIFILE, "Can't open %s", fname);
 	}
+	fcntl(fd, F_SETFL, O_RDONLY);
 	if (fd < 10)
 		fd = savefd(fd, fd);
 	setinputfd(fd, flags & INPUT_PUSH_FILE);
-- 
1.6.0.3

Re: [PATCH] [INPUT] exit 127 if command_file is given but doesn't exist

[Herbert Xu]

On Mon, Jun 14, 2010 at 09:54:51AM +0000, Gerrit Pape wrote:
>
> @@ -405,11 +406,13 @@ setinputfile(const char *fname, int flags)
>  	int fd;
>  
>  	INTOFF;
> -	if ((fd = open(fname, O_RDONLY)) < 0) {
> +	if ((fd = open(fname, O_RDWR)) < 0) {

Why should the file be writeable?

Please do not mix multiple changes in one patch.

>  		if (flags & INPUT_NOFILE_OK)
>  			goto out;
> -		sh_error("Can't open %s", fname);
> +		exitstatus = 127;
> +		exerror(EXIFILE, "Can't open %s", fname);
>  	}
> +	fcntl(fd, F_SETFL, O_RDONLY);

This is wrong.  You're creating exactly the problem that Jilles
was talking about where dot(1) is returning 127.  This code should
be moved to procargs, which runs only for the "sh script" case.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH] [OPTIONS] Use exit status 127 when the script to run does not exist

[Jonathan Nieder]

Currently dash uses exit status 2 to report failure to run a
script named on the command line that does not exist:

 $ sh nonexistent
 sh: Can't open nonexistent
 $ echo $?
 2

According to SUSv3 and SUSv4[1], the exit code should be 127.

So check errno when opening a command_file during argument
processing and exit(127) when it is ENOENT.  This check is in the
argument processing code to avoid collateral damage to code paths
that read scripts (such as the dot builtin).  Other errors (such
as EACCESS) while opening the command_file will still result in
an exit code of 2.

[1] http://www.opengroup.org/onlinepubs/9699919799/utilities/sh.html#tag_20_117_14

Reported-by: Clint Adams <schizo@debian.org>
Fixes: http://bugs.debian.org/548743
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Herbert Xu wrote:

> This is wrong.  You're creating exactly the problem that Jilles
> was talking about where dot(1) is returning 127.  This code should
> be moved to procargs, which runs only for the "sh script" case.

Makes sense.  How does this look?

It still ignores attempts to run "sh directory"; see the next
patch for that.

 src/options.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/src/options.c b/src/options.c
index 6f381e6..5667b68 100644
--- a/src/options.c
+++ b/src/options.c
@@ -37,6 +37,7 @@
 #include <stdlib.h>
 
 #include "shell.h"
+#include "main.h"
 #define DEFINE_OPTIONS
 #include "options.h"
 #undef DEFINE_OPTIONS
@@ -156,7 +157,13 @@ procargs(int argc, char **argv)
 		if (*xargv)
 			goto setarg0;
 	} else if (!sflag) {
-		setinputfile(*xargv, 0);
+		if (setinputfile(*xargv, INPUT_NOFILE_OK) < 0) {
+			if (errno == ENOENT) {
+				exitstatus = 127;
+				exerror(EXEXIT, "Can't open %s", *xargv);
+			}
+			sh_error("Can't open %s", *xargv);
+		}
 setarg0:
 		arg0 = *xargv++;
 		commandname = arg0;
-- 
1.7.2.3

[PATCH] [INPUT] Catch attempts to run a directory as a script

[Jonathan Nieder]

open() succeeds, though read() fails later, when a pathname
passed as argv[0] on the command line points to a directory,
which somehow results in dash thinking everything is okay:

 $ sh dir/
 $ echo $?
 0

But POSIX makes it clear enough that in "sh command_file",
command_file is supposed to be a file, not a directory.  So
diagnose this with an error message and exit with status 2.

Reported-by: Jari Aalto <jari.aalto@cante.net>
Fixes: http://bugs.debian.org/548687
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Applies on top of the previous patch, which is based on
master.

Thanks to Gerrit and Krzysztof for your help so far.

 src/input.c |   21 ++++++++++++++++++++-
 1 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/src/input.c b/src/input.c
index e57ad76..d68d441 100644
--- a/src/input.c
+++ b/src/input.c
@@ -395,6 +395,19 @@ popstring(void)
 }
 
+/*
+ * Is fd a directory file descriptor?
+ */
+
+static int
+isdir(const char *fname, int fd)
+{
+	struct stat64 statb;
+	if (fstat64(fd, &statb) < 0)
+		sh_error("Can't stat %s", fname);
+	return S_ISDIR(statb.st_mode);
+}
+
 /*
  * Set the input to take input from a file.  If push is set, push the
  * old input onto the stack first.
  */
@@ -405,7 +418,13 @@ setinputfile(const char *fname, int flags)
 	int fd;
 
 	INTOFF;
-	if ((fd = open(fname, O_RDONLY)) < 0) {
+	fd = open(fname, O_RDONLY);
+	if (isdir(fname, fd)) {
+		close(fd);
+		fd = -1;
+		errno = EISDIR;
+	}
+	if (fd < 0) {
 		if (flags & INPUT_NOFILE_OK)
 			goto out;
 		sh_error("Can't open %s", fname);
-- 
1.7.2.3

Re: [PATCH] [INPUT] Catch attempts to run a directory as a script

[Herbert Xu]

On Wed, Oct 06, 2010 at 05:08:04AM -0500, Jonathan Nieder wrote:
> open() succeeds, though read() fails later, when a pathname
> passed as argv[0] on the command line points to a directory,
> which somehow results in dash thinking everything is okay:
> 
>  $ sh dir/
>  $ echo $?
>  0
> 
> But POSIX makes it clear enough that in "sh command_file",
> command_file is supposed to be a file, not a directory.  So
> diagnose this with an error message and exit with status 2.
> 
> Reported-by: Jari Aalto <jari.aalto@cante.net>
> Fixes: http://bugs.debian.org/548687
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Is this required by POSIX? If not this is simply making dash
bigger for no good reason.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] [INPUT] Catch attempts to run a directory as a script

[Jonathan Nieder]

Herbert Xu wrote:
> On Wed, Oct 06, 2010 at 05:08:04AM -0500, Jonathan Nieder wrote:

>> But POSIX makes it clear enough that in "sh command_file",
>> command_file is supposed to be a file, not a directory.  So
>> diagnose this with an error message and exit with status 2.
[...]
> Is this required by POSIX? If not this is simply making dash
> bigger for no good reason.

Not clear.  I suppose POSIX usually doesn't require anything when the
caller screws up.

Jari Aalto quoted the DESCRIPTION section[1] (and some similar
passages):

>    The sh utility is a command language interpreter that shall
>    execute commands read from a command line string, the standard
> >> input, or a specified file. 

I don't find that alone very convincing.

Under OPERANDS[2]: if the path contains a slash, all the standard says
is "the implementation attempts to read that file".  If the path does
not contain a slash and the file is not in the working directory, the
implementation _may_ perform a search as described in "Command Search
and Execution".

During that search, after execve() fails, "if the executable file is
not a text file, the shell _may_ bypass this command execution. In
this case, it shall write an error message, and shall return an exit
status of 126." (emphasis mine).

So this behavior is allowed as an optional subset of an optional
behavior.  That may have guided the bash implementors:

 $ bash directory
 directory: directory: is a directory
 $ echo $?
 126

It's probably not required.

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=10;bug=548687
[2] http://www.opengroup.org/onlinepubs/9699919799/utilities/sh.html#tag_20_117_05

Re: [PATCH] [INPUT] Catch attempts to run a directory as a script

[Eric Blake]

On 10/06/2010 04:55 AM, Jonathan Nieder wrote:
>>> But POSIX makes it clear enough that in "sh command_file",
>>> command_file is supposed to be a file, not a directory.  So
>>> diagnose this with an error message and exit with status 2.
> [...]
>> Is this required by POSIX? If not this is simply making dash
>> bigger for no good reason.
>
> Not clear.  I suppose POSIX usually doesn't require anything when the
> caller screws up.

POSIX requires that input files to bash shall be text files; directories 
do not qualify for this.
http://www.opengroup.org/onlinepubs/9699919799/utilities/sh.html
"The input file shall be a text file, except that line lengths shall be 
unlimited. "

However, that is a requirement on the user, not the shell; so running 
'sh /' is a constraint violation by the user, and leaves behavior up to 
the shell.

> Under OPERANDS[2]: if the path contains a slash, all the standard says
> is "the implementation attempts to read that file".  If the path does
> not contain a slash and the file is not in the working directory, the
> implementation _may_ perform a search as described in "Command Search
> and Execution".

It's more than just MAY; it's a requirement:
http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01

"If the command name contains at least one <slash>, the shell shall 
execute the utility in a separate utility environment with actions 
equivalent to calling the execve() function...

"If the execve() function fails due to an error equivalent to the 
[ENOEXEC] error, the shell shall execute a command equivalent to having 
a shell invoked with the command name as its first operand"

>
> During that search, after execve() fails, "if the executable file is
> not a text file, the shell _may_ bypass this command execution. In
> this case, it shall write an error message, and shall return an exit
> status of 126." (emphasis mine).

But yes, that same section is clear that for both command searches along 
PATH for a word without slash, and for a direct command with a slash, if 
execve() fails with ENOEXEC (as it does for directories), then it is 
optional whether the shell bypasses attempts to read the file because it 
was not a text file.

On the other hand, in Linux, execve(".",...) fails with EACCES, as 
permitted by the standard:

http://www.opengroup.org/onlinepubs/9699919799/functions/execve.html
"[EACCES] ...or the new process image file is not a regular file and the 
implementation does not support execution of files of its type."

And since EACCES is not the same class as ENOEXEC, there is no 
requirement for the shell to attempt to execute the same file.  So, 
rather than stat()ing the argument in advance and checking for S_ISDIR, 
it seems like it would be simpler to check after the execve() attempt 
for EACCES and blindly set $? to 126 in that case (since you already 
have to check for ENOEXEC).

> So this behavior is allowed as an optional subset of an optional
> behavior.  That may have guided the bash implementors:
>
>   $ bash directory
>   directory: directory: is a directory
>   $ echo $?
>   126
>
> It's probably not required.

Additionally, the standard REQUIRES that 'sh -c "exec /"' shall fail 
with status 126:

http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#exec
"If command is found, but it is not an executable utility, the exit 
status shall be 126."

Right now, dash gets this wrong:

dash -c 'exec .'; echo $?
exec: 1: /: Permission denied
2

And since you already have the code in dash to detect failure to 'exec' 
a directory, you should be able to reuse that code when detecting 
failure to run a directory as a script, as in 'dash .'.

[Hmm, bash also gets it wrong:
bash -c 'exec .'; echo $?
bash: line 0: exec: .: not found
127
even though . should always be found]

-- 
Eric Blake   eblake@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Re: [PATCH] [INPUT] Catch attempts to run a directory as a script

[Herbert Xu]

On Wed, Oct 06, 2010 at 06:18:05AM -0600, Eric Blake wrote:
>
> Right now, dash gets this wrong:
>
> dash -c 'exec .'; echo $?
> exec: 1: /: Permission denied
> 2
>
> And since you already have the code in dash to detect failure to 'exec'  
> a directory, you should be able to reuse that code when detecting  
> failure to run a directory as a script, as in 'dash .'.

Thanks for the detailed investiagtion!

If someone could send me a patch that does this without bloating
dash too much then I can apply it without feeling too much guilt :)

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 0/3] Fix exit status for 'exec nonexistent' and 'exec .'

[Jonathan Nieder]

Eric Blake wrote:

> Additionally, the standard REQUIRES that 'sh -c "exec /"' shall fail
> with status 126:
> 
> http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#exec
> "If command is found, but it is not an executable utility, the exit
> status shall be 126."
> 
> Right now, dash gets this wrong:
> 
> dash -c 'exec .'; echo $?
> exec: 1: /: Permission denied
> 2
> 
> And since you already have the code in dash to detect failure to
> 'exec' a directory, you should be able to reuse that code when
> detecting failure to run a directory as a script, as in 'dash .'.

Summary:

   Command		Status	Expected status
1) exec .		2	126
2) exec nonexistent	2	127
3) sh .			0	126
4) sh nonexistent	2	127

(1) and (2) seem to be regressions from about 5 years ago.
Reverting the problematic patch fixes it for me; see below.

(3) is not clearly documented in the standard, and there is
some disagreement between shells on how to handle it[1].  The
current behavior seems fine to me.

(4) is a clear bug, well documented in the EXIT STATUS section
of the description of sh in the "Utilities" chapter.  See
http://thread.gmane.org/gmane.comp.shells.dash/291/focus=390
for a patch[2].

Thoughts?
Jonathan Nieder (3):
  [EXCEPTIONS] Stop documenting EXSHELLPROC
  Revert "Eliminated global exerrno."
  [EXCEPTIONS] Eliminate global exerrno

 src/TOUR    |   11 +----------
 src/error.h |    5 ++---
 src/eval.c  |    8 ++++----
 3 files changed, 7 insertions(+), 17 deletions(-)

[1]
$ bash .; echo $?
.: .: is a directory
126
$ ksh93 .; echo $?
ksh93: .: cannot open [Is a directory]
126
$ pdksh .; echo $?
0

[2] Unfortunately this does not share code with the fix to (1)
and (2) as Eric hoped.  Why?  The system calls involved are
different: the exec builtin uses execve(), while 'sh <script>'
uses open() and read().

[PATCH 1/3] [EXCEPTIONS] Stop documenting EXSHELLPROC

[Jonathan Nieder]

At some point between ash 0.3.5-11.0.1 and ash 0.3.8-37, Debian
ash stopped using the EXSHELLPROC exception to handle shell
scripts without a magic number.

Remove all remaining references to it to avoid confusion.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 src/TOUR    |   11 +----------
 src/error.h |    5 ++---
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/src/TOUR b/src/TOUR
index 4baac62..056e79b 100644
--- a/src/TOUR
+++ b/src/TOUR
@@ -43,10 +43,6 @@ C source files for entries looking like:
                            back to the main command loop */
         }
 
-        SHELLPROC {
-              x = 3;    /* executed when the shell runs a shell procedure */
-        }
-
 It pulls this code out into routines which are when particular
 events occur.  The intent is to improve modularity by isolating
 the information about which modules need to be explicitly
@@ -79,12 +75,7 @@ EXCEPTIONS:  Code for dealing with exceptions appears in
 exceptions.c.  The C language doesn't include exception handling,
 so I implement it using setjmp and longjmp.  The global variable
 exception contains the type of exception.  EXERROR is raised by
-calling error.  EXINT is an interrupt.  EXSHELLPROC is an excep-
-tion which is raised when a shell procedure is invoked.  The pur-
-pose of EXSHELLPROC is to perform the cleanup actions associated
-with other exceptions.  After these cleanup actions, the shell
-can interpret a shell procedure itself without exec'ing a new
-copy of the shell.
+calling error.  EXINT is an interrupt.
 
 INTERRUPTS:  In an interactive shell, an interrupt will cause an
 EXINT exception to return to the main command loop.  (Exception:
diff --git a/src/error.h b/src/error.h
index 3162e15..15ccbe3 100644
--- a/src/error.h
+++ b/src/error.h
@@ -66,9 +66,8 @@ extern int exception;
 /* exceptions */
 #define EXINT 0		/* SIGINT received */
 #define EXERROR 1	/* a generic error */
-#define EXSHELLPROC 2	/* execute a shell procedure */
-#define EXEXEC 3	/* command execution failed */
-#define EXEXIT 4	/* exit the shell */
+#define EXEXEC 2	/* command execution failed */
+#define EXEXIT 3	/* exit the shell */
 
 
 /*
-- 
1.7.2.3

Re: [PATCH 1/3] [EXCEPTIONS] Stop documenting EXSHELLPROC

[Herbert Xu]

On Wed, Oct 06, 2010 at 08:03:47PM -0500, Jonathan Nieder wrote:
>
> diff --git a/src/error.h b/src/error.h
> index 3162e15..15ccbe3 100644
> --- a/src/error.h
> +++ b/src/error.h
> @@ -66,9 +66,8 @@ extern int exception;
>  /* exceptions */
>  #define EXINT 0		/* SIGINT received */
>  #define EXERROR 1	/* a generic error */
> -#define EXSHELLPROC 2	/* execute a shell procedure */
> -#define EXEXEC 3	/* command execution failed */
> -#define EXEXIT 4	/* exit the shell */
> +#define EXEXEC 2	/* command execution failed */
> +#define EXEXIT 3	/* exit the shell */

There is no need to renumber things.  Just leave a gap.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 1/3] [EXCEPTIONS] Stop documenting EXSHELLPROC

[Jonathan Nieder]

Herbert Xu wrote:
> On Wed, Oct 06, 2010 at 08:03:47PM -0500, Jonathan Nieder wrote:

>> --- a/src/error.h
>> +++ b/src/error.h
>> @@ -66,9 +66,8 @@ extern int exception;
>>  /* exceptions */
>>  #define EXINT 0		/* SIGINT received */
>>  #define EXERROR 1	/* a generic error */
>> -#define EXSHELLPROC 2	/* execute a shell procedure */
>> -#define EXEXEC 3	/* command execution failed */
>> -#define EXEXIT 4	/* exit the shell */
>> +#define EXEXEC 2	/* command execution failed */
>> +#define EXEXIT 3	/* exit the shell */
>
> There is no need to renumber things.  Just leave a gap.

Ok.  Should I fix it up or would you like to?

Thanks,
Jonathan

Re: [PATCH 1/3] [EXCEPTIONS] Stop documenting EXSHELLPROC

[Herbert Xu]

On Wed, Oct 06, 2010 at 10:04:14PM -0500, Jonathan Nieder wrote:
> Herbert Xu wrote:
> > On Wed, Oct 06, 2010 at 08:03:47PM -0500, Jonathan Nieder wrote:
> 
> >> --- a/src/error.h
> >> +++ b/src/error.h
> >> @@ -66,9 +66,8 @@ extern int exception;
> >>  /* exceptions */
> >>  #define EXINT 0		/* SIGINT received */
> >>  #define EXERROR 1	/* a generic error */
> >> -#define EXSHELLPROC 2	/* execute a shell procedure */
> >> -#define EXEXEC 3	/* command execution failed */
> >> -#define EXEXIT 4	/* exit the shell */
> >> +#define EXEXEC 2	/* command execution failed */
> >> +#define EXEXIT 3	/* exit the shell */
> >
> > There is no need to renumber things.  Just leave a gap.
> 
> Ok.  Should I fix it up or would you like to?

Yeah please resend the patch with the fix-up.

Thanks!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH v2] [EXCEPTIONS] Stop documenting EXSHELLPROC

[Jonathan Nieder]

At some point between ash 0.3.5-11.0.1 and ash 0.3.8-37, Debian
ash stopped using the EXSHELLPROC exception to handle shell
scripts without a magic number.

Remove all remaining references to it to avoid confusion.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Herbert Xu wrote:

> Yeah please resend the patch with the fix-up.

Done.  (Patch is against master, as before.)

 src/TOUR    |   11 +----------
 src/error.h |    1 -
 2 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/src/TOUR b/src/TOUR
index 4baac62..056e79b 100644
--- a/src/TOUR
+++ b/src/TOUR
@@ -43,10 +43,6 @@ C source files for entries looking like:
                            back to the main command loop */
         }
 
-        SHELLPROC {
-              x = 3;    /* executed when the shell runs a shell procedure */
-        }
-
 It pulls this code out into routines which are when particular
 events occur.  The intent is to improve modularity by isolating
 the information about which modules need to be explicitly
@@ -79,12 +75,7 @@ EXCEPTIONS:  Code for dealing with exceptions appears in
 exceptions.c.  The C language doesn't include exception handling,
 so I implement it using setjmp and longjmp.  The global variable
 exception contains the type of exception.  EXERROR is raised by
-calling error.  EXINT is an interrupt.  EXSHELLPROC is an excep-
-tion which is raised when a shell procedure is invoked.  The pur-
-pose of EXSHELLPROC is to perform the cleanup actions associated
-with other exceptions.  After these cleanup actions, the shell
-can interpret a shell procedure itself without exec'ing a new
-copy of the shell.
+calling error.  EXINT is an interrupt.
 
 INTERRUPTS:  In an interactive shell, an interrupt will cause an
 EXINT exception to return to the main command loop.  (Exception:
diff --git a/src/error.h b/src/error.h
index 3162e15..be0eec9 100644
--- a/src/error.h
+++ b/src/error.h
@@ -66,7 +66,6 @@ extern int exception;
 /* exceptions */
 #define EXINT 0		/* SIGINT received */
 #define EXERROR 1	/* a generic error */
-#define EXSHELLPROC 2	/* execute a shell procedure */
 #define EXEXEC 3	/* command execution failed */
 #define EXEXIT 4	/* exit the shell */
 
-- 
1.7.2.3

Re: [PATCH v2] [EXCEPTIONS] Stop documenting EXSHELLPROC

[Herbert Xu]

On Wed, Oct 06, 2010 at 10:39:23PM -0500, Jonathan Nieder wrote:
> At some point between ash 0.3.5-11.0.1 and ash 0.3.8-37, Debian
> ash stopped using the EXSHELLPROC exception to handle shell
> scripts without a magic number.
> 
> Remove all remaining references to it to avoid confusion.
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Also applied.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 2/3] Revert "Eliminated global exerrno."

[Jonathan Nieder]

This reverts commit c0e07c010e5abdea1a7d1357edb1d08adac529cb.

Yes, it is nicer to pass the exit status through the exitstatus
global, but look at the consequences:

 $ sh -c 'exec nonexistent'; echo $?
 exec: 1: nonexistent: not found
 2
 $ sh -c 'exec .'; echo $?
 exec: 1: .: Permission denied
 2

According to POSIX:
"If command is not found, the exit status shall be 127. If command is
found, but it is not an executable utility, the exit status shall
be 126."

Make it so again.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 src/error.h |    1 +
 src/exec.c  |    3 +--
 src/main.c  |    2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/error.h b/src/error.h
index 15ccbe3..1bb6a82 100644
--- a/src/error.h
+++ b/src/error.h
@@ -62,6 +62,7 @@ struct jmploc {
 
 extern struct jmploc *handler;
 extern int exception;
+extern int exerrno;	/* error for EXEXEC */
 
 /* exceptions */
 #define EXINT 0		/* SIGINT received */
diff --git a/src/exec.c b/src/exec.c
index 42299ea..9525a16 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -86,6 +86,7 @@ struct tblentry {
 
 STATIC struct tblentry *cmdtable[CMDTABLESIZE];
 STATIC int builtinloc = -1;		/* index in path of %builtin, or -1 */
+int exerrno;				/* Last exec error */
 
 
 STATIC void tryexec(char *, char **, char **);
@@ -108,7 +109,6 @@ shellexec(char **argv, const char *path, int idx)
 	char *cmdname;
 	int e;
 	char **envp;
-	int exerrno;
 
 	envp = environment();
 	if (strchr(argv[0], '/') != NULL) {
@@ -138,7 +138,6 @@ shellexec(char **argv, const char *path, int idx)
 		exerrno = 2;
 		break;
 	}
-	exitstatus = exerrno;
 	TRACE(("shellexec failed for %s, errno %d, suppressint %d\n",
 		argv[0], e, suppressint ));
 	exerror(EXEXEC, "%s: %s", argv[0], errmsg(e, E_EXEC));
diff --git a/src/main.c b/src/main.c
index 2bff956..5bc1a99 100644
--- a/src/main.c
+++ b/src/main.c
@@ -115,6 +115,8 @@ main(int argc, char **argv)
 		e = exception;
 		if (e == EXERROR)
 			exitstatus = 2;
+		else if (e == EXEXEC)
+			exitstatus = exerrno;
 
 		s = state;
 		if (e == EXEXIT || s == 0 || iflag == 0 || shlvl)
-- 
1.7.2.3

Re: [PATCH 2/3] Revert "Eliminated global exerrno."

[Herbert Xu]

On Wed, Oct 06, 2010 at 08:04:58PM -0500, Jonathan Nieder wrote:
> This reverts commit c0e07c010e5abdea1a7d1357edb1d08adac529cb.
> 
> Yes, it is nicer to pass the exit status through the exitstatus
> global, but look at the consequences:
> 
>  $ sh -c 'exec nonexistent'; echo $?
>  exec: 1: nonexistent: not found
>  2
>  $ sh -c 'exec .'; echo $?
>  exec: 1: .: Permission denied
>  2
> 
> According to POSIX:
> "If command is not found, the exit status shall be 127. If command is
> found, but it is not an executable utility, the exit status shall
> be 126."
> 
> Make it so again.
> 
> Reported-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Actually the bug is elsewhere.  This patch works for me.

commit 7f684260a2426ac61c06d2e4822429b00437ae24
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Thu Oct 7 10:55:15 2010 +0800

    [BUILTIN] Fix EXEXEC status clobbering
    
    evalcommand always clobbers the exit status in case of an EXEXEC
    which means that exec always fails with exit status 2 regardless
    of what it actually returns.
    
    This patch adds the missing check for EXEXEC so that the correct
    exit status is preserved.
    
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/ChangeLog b/ChangeLog
index 1dfe241..2faaedd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2010-10-07  Herbert Xu <herbert@gondor.apana.org.au>
+
+	* Fix EXEXEC status clobbering.
+
 2010-09-08  Herbert Xu <herbert@gondor.apana.org.au>
 
 	* Fix ifsfirst/ifslastp leak.
diff --git a/src/eval.c b/src/eval.c
index 5b8d36b..b966749 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -854,7 +854,7 @@ bail:
 			int i;
 
 			i = exception;
-			if (i == EXEXIT)
+			if (i == EXEXIT || i == EXEXEC)
 				goto raise;
 
 			status = (i == EXINT) ? SIGINT + 128 : 2;

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/3] Revert "Eliminated global exerrno."

[Jonathan Nieder]

Herbert Xu wrote:

> Actually the bug is elsewhere.

It does bisect to there. :)  But you're right, it would have been
simpler to send one patch.

> --- a/src/eval.c
> +++ b/src/eval.c
> @@ -854,7 +854,7 @@ bail:
>  			int i;
>  
>  			i = exception;
> -			if (i == EXEXIT)
> +			if (i == EXEXIT || i == EXEXEC)
>  				goto raise;

Good call.  This is better than my patch because it exits like it
ought to for

	command exec nonexistent

(as POSIX says:

	If command is specified, exec shall not return to the shell

).

Maybe the following would make sense on top?

-- 8< --
Subject: [EXCEPTIONS] Use EXEXIT in place of EXEXEC

The intended semantics of EXEXEC are identical to EXEXIT, so
simplify by using EXEXIT directly.

Functional change: in edge cases (exec within a trap handler),
this causes the exit status from exec not to be clobbered.
For example, without this patch:

 $ sh -c 'trap "exec nonexistent" EXIT'; echo $?
 exec: 1: nonexistent: not found
 0

And with it:

 $ sh -c 'trap "exec nonexistent" EXIT'; echo $?
 exec: 1: nonexistent: not found
 127

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 src/error.h |    1 -
 src/eval.c  |    2 +-
 src/exec.c  |    2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/error.h b/src/error.h
index be0eec9..f236d9f 100644
--- a/src/error.h
+++ b/src/error.h
@@ -66,7 +66,6 @@ extern int exception;
 /* exceptions */
 #define EXINT 0		/* SIGINT received */
 #define EXERROR 1	/* a generic error */
-#define EXEXEC 3	/* command execution failed */
 #define EXEXIT 4	/* exit the shell */
 
 
diff --git a/src/eval.c b/src/eval.c
index b966749..5b8d36b 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -854,7 +854,7 @@ bail:
 			int i;
 
 			i = exception;
-			if (i == EXEXIT || i == EXEXEC)
+			if (i == EXEXIT)
 				goto raise;
 
 			status = (i == EXINT) ? SIGINT + 128 : 2;
diff --git a/src/exec.c b/src/exec.c
index 42299ea..b273420 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -141,7 +141,7 @@ shellexec(char **argv, const char *path, int idx)
 	exitstatus = exerrno;
 	TRACE(("shellexec failed for %s, errno %d, suppressint %d\n",
 		argv[0], e, suppressint ));
-	exerror(EXEXEC, "%s: %s", argv[0], errmsg(e, E_EXEC));
+	exerror(EXEXIT, "%s: %s", argv[0], errmsg(e, E_EXEC));
 	/* NOTREACHED */
 }
 
-- 
1.7.2.3

Re: [PATCH 2/3] Revert "Eliminated global exerrno."

[Herbert Xu]

On Wed, Oct 06, 2010 at 10:35:18PM -0500, Jonathan Nieder wrote:
> Herbert Xu wrote:
> 
> > Actually the bug is elsewhere.
> 
> It does bisect to there. :)  But you're right, it would have been
> simpler to send one patch.
> 
> > --- a/src/eval.c
> > +++ b/src/eval.c
> > @@ -854,7 +854,7 @@ bail:
> >  			int i;
> >  
> >  			i = exception;
> > -			if (i == EXEXIT)
> > +			if (i == EXEXIT || i == EXEXEC)
> >  				goto raise;
> 
> Good call.  This is better than my patch because it exits like it
> ought to for
> 
> 	command exec nonexistent
> 
> (as POSIX says:
> 
> 	If command is specified, exec shall not return to the shell
> 
> ).
> 
> Maybe the following would make sense on top?

Nice! I like where you are going with this :)

Let me run it through the tests and apply it.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/3] Revert "Eliminated global exerrno."

[Herbert Xu]

On Thu, Oct 07, 2010 at 12:14:06PM +0800, Herbert Xu wrote:
>
> Let me run it through the tests and apply it.

There is one problem with this and that is changing the exit
status in a trap:

	sh -c 'trap "exec nosuchfile" EXIT; exit 3'; echo $?

This makes dash exit with 127 instead of 3 as it does now.

AFAIK the wording of POSIX is such that we should return 3, however,
it does seem that every other shell exits with 127.

So unless anyone objects, I will apply your patch and adopt this
new behaviour.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/3] Revert "Eliminated global exerrno."

[Jonathan Nieder]

Herbert Xu wrote:

> 	sh -c 'trap "exec nosuchfile" EXIT; exit 3'; echo $?
> 
> This makes dash exit with 127 instead of 3 as it does now.
> 
> AFAIK the wording of POSIX is such that we should return 3, however,
> it does seem that every other shell exits with 127.

On 'exec':

 "If exec is specified with command, it shall replace the shell with
 command without creating a new process."		(1)

 "If command is specified, exec shall not return to the shell; rather,
 the exit status of the process shall be the exit status of the program
 implementing command, which overlaid the shell. If command is not
 found, the exit status shall be 127. If command is found, but it is
 not an executable utility, the exit status shall be 126. If a
 redirection error occurs (see Consequences of Shell Errors ), the
 shell shall exit with a value in the range 1-125. Otherwise, exec
 shall return a zero exit status."			(2)

On 'exit':

 "...
 A trap on EXIT shall be executed before the shell terminates, except
 when the exit utility is invoked in that trap itself, in which case
 the shell shall exit immediately."			(3)

 "The exit status [of the exit utility -jrn] shall be n, if specified.
 Otherwise, the value shall be the exit value of the last command
 executed, or zero if no command was executed. When exit is executed in
 a trap action, the last command is considered to be the command that
 executed immediately preceding the trap action."	(4)

The passage (3) seems to mean that "exit" triggers an EXIT trap
(except, to avoid having to deal with recursion, when the "exit"
occurs within an EXIT trap).  Then the exec builtin is invoked,
resulting (according to (2)) in an exit(127) without returning to the
shell.

So I think the proposed behavior fits the spec.

Regards,
Jonathan

Re: [PATCH 2/3] Revert "Eliminated global exerrno."

[Herbert Xu]

On Wed, Oct 06, 2010 at 10:35:18PM -0500, Jonathan Nieder wrote:
>
> Maybe the following would make sense on top?
> 
> -- 8< --
> Subject: [EXCEPTIONS] Use EXEXIT in place of EXEXEC

Patch applied.  Thanks a lot!
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 3/3] [EXCEPTIONS] Eliminate global exerrno

[Jonathan Nieder]

Pass the exit status from the exec builtin through the exitstatus
global.  This just eliminates a global variable; no noticeable
change is intended.

Based on v0.5.3~57 (Eliminated global exerrno, 2005-02-25).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That's the end of the series.  Thanks for reading.

 src/error.h |    1 -
 src/eval.c  |    8 ++++----
 src/exec.c  |    3 ++-
 src/main.c  |    2 --
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/error.h b/src/error.h
index 1bb6a82..15ccbe3 100644
--- a/src/error.h
+++ b/src/error.h
@@ -62,7 +62,6 @@ struct jmploc {
 
 extern struct jmploc *handler;
 extern int exception;
-extern int exerrno;	/* error for EXEXEC */
 
 /* exceptions */
 #define EXINT 0		/* SIGINT received */
diff --git a/src/eval.c b/src/eval.c
index 5b8d36b..0406571 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -850,15 +850,15 @@ bail:
 				listsetvar(varlist.list, VEXPORT);
 		}
 		if (evalbltin(cmdentry.u.cmd, argc, argv, flags)) {
-			int status;
 			int i;
 
 			i = exception;
 			if (i == EXEXIT)
 				goto raise;
-
-			status = (i == EXINT) ? SIGINT + 128 : 2;
-			exitstatus = status;
+			if (i == EXINT)
+				exitstatus = SIGINT + 128;
+			if (i == EXERROR)
+				exitstatus = 2;
 
 			if (i == EXINT || spclbltin > 0) {
 raise:
diff --git a/src/exec.c b/src/exec.c
index 9525a16..42299ea 100644
--- a/src/exec.c
+++ b/src/exec.c
@@ -86,7 +86,6 @@ struct tblentry {
 
 STATIC struct tblentry *cmdtable[CMDTABLESIZE];
 STATIC int builtinloc = -1;		/* index in path of %builtin, or -1 */
-int exerrno;				/* Last exec error */
 
 
 STATIC void tryexec(char *, char **, char **);
@@ -109,6 +108,7 @@ shellexec(char **argv, const char *path, int idx)
 	char *cmdname;
 	int e;
 	char **envp;
+	int exerrno;
 
 	envp = environment();
 	if (strchr(argv[0], '/') != NULL) {
@@ -138,6 +138,7 @@ shellexec(char **argv, const char *path, int idx)
 		exerrno = 2;
 		break;
 	}
+	exitstatus = exerrno;
 	TRACE(("shellexec failed for %s, errno %d, suppressint %d\n",
 		argv[0], e, suppressint ));
 	exerror(EXEXEC, "%s: %s", argv[0], errmsg(e, E_EXEC));
diff --git a/src/main.c b/src/main.c
index 5bc1a99..2bff956 100644
--- a/src/main.c
+++ b/src/main.c
@@ -115,8 +115,6 @@ main(int argc, char **argv)
 		e = exception;
 		if (e == EXERROR)
 			exitstatus = 2;
-		else if (e == EXEXEC)
-			exitstatus = exerrno;
 
 		s = state;
 		if (e == EXEXIT || s == 0 || iflag == 0 || shlvl)
-- 
1.7.2.3

Re: [PATCH 3/3] [EXCEPTIONS] Eliminate global exerrno

[Herbert Xu]

On Wed, Oct 06, 2010 at 08:08:07PM -0500, Jonathan Nieder wrote:
> Pass the exit status from the exec builtin through the exitstatus
> global.  This just eliminates a global variable; no noticeable
> change is intended.
> 
> Based on v0.5.3~57 (Eliminated global exerrno, 2005-02-25).
> 
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

Heh, had you sent 2/3 and 3/3 as one patch I would've seen the
problem and just applied it :)

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] [OPTIONS] Use exit status 127 when the script to run does not exist

[Herbert Xu]

On Wed, Oct 06, 2010 at 05:04:20AM -0500, Jonathan Nieder wrote:
>
> Herbert Xu wrote:
> 
> > This is wrong.  You're creating exactly the problem that Jilles
> > was talking about where dot(1) is returning 127.  This code should
> > be moved to procargs, which runs only for the "sh script" case.
> 
> Makes sense.  How does this look?
> 
> It still ignores attempts to run "sh directory"; see the next
> patch for that.

Actually I was wrong.  I had misunderstood Jilles' report and
thought that the problem is with 127 coming from dotcmd.  His
problem is actually with dotcmd bailing out which your (or was
it Gerrit's?) EXFILE patch already addressed.

However, instead of adding an ad-hoc exception for EXFILE, I'm
simply going to move the exit status setting for EXERROR out of
main.c and into error.c which should let you get what you want.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] [OPTIONS] Use exit status 127 when the script to run does not exist

[Herbert Xu]

On Sun, Nov 28, 2010 at 08:06:27PM +0800, Herbert Xu wrote:
>
> However, instead of adding an ad-hoc exception for EXFILE, I'm
> simply going to move the exit status setting for EXERROR out of
> main.c and into error.c which should let you get what you want.

Something like this:

commit a42317f10233cf2fcff033cc705fd14315188374
Author: Herbert Xu <herbert@gondor.apana.org.au>
Date:   Sun Nov 28 20:22:00 2010 +0800

    [ERROR] Allow the originator of EXERROR to set the exit status
    
    Some errors have exit status values specified by POSIX and it is
    therefore desirable to be able to set the exit status at the EXERROR
    source rather than in main.c.
    
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/ChangeLog b/ChangeLog
index 76cbeb4..e5479a8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,7 @@
 2010-11-28  Herbert Xu <herbert@gondor.apana.org.au>
 
 	* Fixed trap/return regression due to SKIPEVAL removal.
+	* Allow the originator of EXERROR to set the exit status.
 
 2010-10-18  Herbert Xu <herbert@gondor.apana.org.au>
 
diff --git a/src/error.c b/src/error.c
index e304d3d..f1a358d 100644
--- a/src/error.c
+++ b/src/error.c
@@ -163,6 +163,8 @@ sh_error(const char *msg, ...)
 {
 	va_list ap;
 
+	exitstatus = 2;
+
 	va_start(ap, msg);
 	exverror(EXERROR, msg, ap);
 	/* NOTREACHED */
diff --git a/src/eval.c b/src/eval.c
index ea4afc6..013a8dd 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -823,11 +823,13 @@ evalcommand(union node *cmd, int flags)
 	}
 
 	if (status) {
+bail:
+		exitstatus = status;
+
 		/* We have a redirection error. */
 		if (spclbltin > 0)
 			exraise(EXERROR);
-bail:
-		exitstatus = status;
+
 		goto out;
 	}
 
diff --git a/src/main.c b/src/main.c
index 2bff956..1735c67 100644
--- a/src/main.c
+++ b/src/main.c
@@ -113,8 +113,6 @@ main(int argc, char **argv)
 		reset();
 
 		e = exception;
-		if (e == EXERROR)
-			exitstatus = 2;
 
 		s = state;
 		if (e == EXEXIT || s == 0 || iflag == 0 || shlvl)

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] [OPTIONS] Use exit status 127 when the script to run does not exist

[Herbert Xu]

On Sun, Nov 28, 2010 at 08:24:37PM +0800, Herbert Xu wrote:
> On Sun, Nov 28, 2010 at 08:06:27PM +0800, Herbert Xu wrote:
> >
> > However, instead of adding an ad-hoc exception for EXFILE, I'm
> > simply going to move the exit status setting for EXERROR out of
> > main.c and into error.c which should let you get what you want.
> 
> Something like this:

And here is the final patch that I applied for this:

commit 34f60a3781ea8f17f3d2a1a743a9aab9b31a296f
Author: Gerrit Pape <pape@smarden.org>
Date:   Sun Nov 28 20:32:00 2010 +0800

    [INPUT] Use exit status 127 when the script to run does not exist
    
    This commit makes dash exit with return code 127 instead of 2 if
    started as non-interactive shell with a non-existent command_file
    specified as argument (or a directory), as documented in
     http://www.opengroup.org/onlinepubs/009695399/utilities/sh.html#tag_04_128_14
    
    The wrong exit code was reported by Clint Adams and Jari Aalto through
     http://bugs.debian.org/548743
     http://bugs.debian.org/548687
    
    Signed-off-by: Gerrit Pape <pape@smarden.org>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/ChangeLog b/ChangeLog
index e5479a8..a18838c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2010-11-28  Gerrit Pape <pape@smarden.org>
+
+	* Use exit status 127 when the script to run does not exist.
+
 2010-11-28  Philipp Weis <pweis@pweis.com>
 
 	* Document optional open parenthesis for case patterns.
diff --git a/src/input.c b/src/input.c
index e57ad76..bd3a9a2 100644
--- a/src/input.c
+++ b/src/input.c
@@ -42,6 +42,7 @@
  * This file implements the input routines used by the parser.
  */
 
+#include "eval.h"
 #include "shell.h"
 #include "redir.h"
 #include "syntax.h"
@@ -408,7 +409,8 @@ setinputfile(const char *fname, int flags)
 	if ((fd = open(fname, O_RDONLY)) < 0) {
 		if (flags & INPUT_NOFILE_OK)
 			goto out;
-		sh_error("Can't open %s", fname);
+		exitstatus = 127;
+		exerror(EXERROR, "Can't open %s", fname);
 	}
 	if (fd < 10)
 		fd = savefd(fd, fd);

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt