[PATCH] return status 126 for execution failures other than 'not found'

[Martijn Dekker <martijn@xxxxxxxx>]

When the argument list to an external command is too long (exceeds 
ARG_MAX), zsh returns exit status 127. This is incorrect because status 
127 means the command was not found.

POSIX says in "2.8.2 Exit Status for Commands":
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_02
| If a command is not found, the exit status shall be 127. If the
| command name is found, but it is not an executable utility, the exit
| status shall be 126.

Now, the phrasing "it is not an executable utility" is a little vague, 
but the current versions of all other shells return 126 upon any failure 
to execute a utility that was found, so they seem to interpret that 
phrase as meaning "it could not be executed for whatever reason (other 
than 'not found')". In any case, 126 is better than any alternative, as 
all other exit codes potentially conflict with other meanings.

Currently, the execute() function in Src/exec.c only returns status 126 
if the error code is EACCES (permission denied) or ENOEXEC (exec format 
error). In all other cases, 127 is returned.

That logic is not right, because 127 is the specific case: it is only to 
be used if the command was not found. Any failure to execute after the 
command is found should yield status 126.

The attached patch changes that logic to return status 127 if there is 
no error number (which happens if a PATH search does not find a command) 
or if the error number is ENOENT (no such file or directory). In all 
other cases it now returns 126.

I've also added a test for the "argument list too long" case.

- Martijn

--
modernish -- harness the shell
https://github.com/modernish/modernish

diff --git a/Src/exec.c b/Src/exec.c
index 2b8e2167f..be949049f 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -726,7 +726,7 @@ execute(LinkList args, int flags, int defpath)
 		(arg0[0] == '.' && (arg0 + 1 == s ||
 				    (arg0[1] == '.' && arg0 + 2 == s)))) {
 		zerr("%e: %s", lerrno, arg0);
-		_exit((lerrno == EACCES || lerrno == ENOEXEC) ? 126 : 127);
+		_exit((lerrno == 0 || lerrno == ENOENT) ? 127 : 126);
 	    }
 	    break;
 	}
@@ -805,7 +805,7 @@ execute(LinkList args, int flags, int defpath)
 	_realexit();
     else
 	zerr("command not found: %s", arg0);
-    _exit((eno == EACCES || eno == ENOEXEC) ? 126 : 127);
+    _exit((eno == 0 || eno == ENOENT) ? 127 : 126);
 }
 
 #define RET_IF_COM(X) { if (iscom(X)) return docopy ? dupstring(X) : arg0; }
diff --git a/Test/A05execution.ztst b/Test/A05execution.ztst
index edc561582..64fdd8288 100644
--- a/Test/A05execution.ztst
+++ b/Test/A05execution.ztst
@@ -394,3 +394,13 @@ F:anonymous function, and a descriptor leak when backgrounding a pipeline
 >127
 # TBD: the 0 above is believed to be bogus and should also be turned
 # into 127 when the ccorresponding bug is fixed in the main shell.
+
+# Test for exit status 126 when an external command's argument list is too long.
+  v=foo_bar_baz_quux_lorem_ipsum_dolor_sit_amet
+  while :; do
+    v=$v$v$v$v$v$v$v$v
+    env true $v || { print $?; break; }
+  done
+0:exit status when argument list too long
+>126
+?(eval):4: argument list too long: env