Re: sh compatibility issue

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On Sun, 20 Feb 2011 16:39:29 -0500
Chet Ramey <chet.ramey@case.edu> wrote:
> On 2/20/11 2:11 PM, Peter Stephenson wrote:
> >> On a related note, here is another quite insidious sh compatibility
> >> issue:
> >>   sh -c 'exec </nonexistent/a; echo wrong'
> >> This should not print "wrong" because exec is a special builtin and
> >> redirection errors on special builtins are fatal. Most shells get this
> >> right nowadays (bash only in POSIX mode) but zsh gets it wrong. Even
> >>   set -o posixbuiltins
> >> does not help.
> > 
> > I think it does need to be fatal, but not because it's a special builtin
> > --- it looks like that's only a "may" rather than a "shall" --- but
> > because there is a rule for exec:
> 
> No, that's actually a `shall'.  Section 2.8.1, special builtin, redirection
> error.

Thanks, didn't follow the link... it looks like the "may" here means
"may be required to".  The table shows essentially all errors for
special builtins are fatal.

I didn't actually test for a special builtin in the previous patch,
either.  Here's a revised patch with more careful tests.

Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.101
diff -p -u -r1.101 options.yo
--- Doc/Zsh/options.yo	21 Feb 2011 11:32:47 -0000	1.101
+++ Doc/Zsh/options.yo	22 Feb 2011 20:00:52 -0000
@@ -1871,9 +1871,9 @@ tt(times),
 tt(trap) and
 tt(unset).
 
-In addition, a failed redirection after tt(exec) causes a non-interactive
-shell to exit and an interactive shell to return to its top-level
-processing.
+In addition, various error conditions associated with the above builtins
+or tt(exec) cause a non-interactive shell to exit and an interactive
+shell to return to its top-level processing.
 )
 pindex(POSIX_IDENTIFIERS)
 pindex(NO_POSIX_IDENTIFIERS)
Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.246
diff -p -u -r1.246 builtin.c
--- Src/builtin.c	7 Jan 2011 10:05:35 -0000	1.246
+++ Src/builtin.c	22 Feb 2011 20:00:53 -0000
@@ -4821,8 +4821,14 @@ bin_dot(char *name, char **argv, UNUSED(
 	freearray(pparams);
 	pparams = old;
     }
-    if (ret == SOURCE_NOT_FOUND)
-	zwarnnam(name, "%e: %s", errno, enam);
+    if (ret == SOURCE_NOT_FOUND) {
+	if (isset(POSIXBUILTINS)) {
+	    /* hard error in POSIX (we'll exit later) */
+	    zerrnam(name, "%e: %s", errno, enam);
+	} else {
+	    zwarnnam(name, "%e: %s", errno, enam);
+	}
+    }
     zsfree(arg0);
     if (old0) {
 	zsfree(argzero);
Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.189
diff -p -u -r1.189 exec.c
--- Src/exec.c	21 Feb 2011 11:28:49 -0000	1.189
+++ Src/exec.c	22 Feb 2011 20:00:53 -0000
@@ -2416,6 +2416,8 @@ execcmd(Estate state, int input, int out
 		checked = !(cflags & BINF_BUILTIN);
 		break;
 	    }
+	    cflags &= ~BINF_BUILTIN & ~BINF_COMMAND;
+	    cflags |= hn->flags;
 	    if (!(hn->flags & BINF_PREFIX)) {
 		is_builtin = 1;
 
@@ -2425,8 +2427,6 @@ execcmd(Estate state, int input, int out
 		assign = (hn->flags & BINF_MAGICEQUALS);
 		break;
 	    }
-	    cflags &= ~BINF_BUILTIN & ~BINF_COMMAND;
-	    cflags |= hn->flags;
 	    checked = 0;
 	    if ((cflags & BINF_COMMAND) && nextnode(firstnode(args))) {
 		/* check for options to command builtin */
@@ -3297,12 +3297,20 @@ execcmd(Estate state, int input, int out
     fixfds(save);
 
  done:
-    if (redir_err && isset(POSIXBUILTINS)) {
-	if (!isset(INTERACTIVE)) {
-	    /* We've already _exit'ed if forked */
-	    exit(1);
+    if (isset(POSIXBUILTINS) &&
+	(cflags & (BINF_PSPECIAL|BINF_EXEC))) {
+	/*
+	 * For POSIX-compatibile behaviour with special
+	 * builtins (including exec which we don't usually
+	 * classify as a builtin, we treat all errors as fatal.
+	 */
+	if (redir_err || errflag) {
+	    if (!isset(INTERACTIVE)) {
+		/* We've already _exit'ed if forked */
+		exit(1);
+	    }
+	    errflag = 1;
 	}
-	errflag = 1;
     }
     if (newxtrerr) {
 	fil = fileno(newxtrerr);
Index: Test/A04redirect.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A04redirect.ztst,v
retrieving revision 1.16
diff -p -u -r1.16 A04redirect.ztst
--- Test/A04redirect.ztst	21 Feb 2011 11:28:49 -0000	1.16
+++ Test/A04redirect.ztst	22 Feb 2011 20:00:53 -0000
@@ -378,3 +378,52 @@
   echo output'
 1:failed exec redir, POSIX_BUILTINS
 ?zsh:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  set >/nonexistent/nonexistent
+  echo output'
+1:failed special builtin redir, POSIX_BUILTINS
+?zsh:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  echo >/nonexistent/nonexistent
+  echo output'
+0:failed unspecial builtin redir, POSIX_BUILTINS
+>output
+?zsh:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS -c '
+  . /nonexistent/nonexistent
+  echo output'
+1:failed dot, POSIX_BUILTINS
+?zsh:.:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f -c '
+  . /nonexistent/nonexistent
+  echo output'
+0:failed dot, NO_POSIX_BUILTINS
+>output
+?zsh:.:2: no such file or directory: /nonexistent/nonexistent
+
+  $ZTST_testdir/../Src/zsh -f <<<'
+  readonly foo
+  foo=bar set output
+  echo output'
+0:failed assignment on posix special, NO_POSIX_BUILTINS
+>output
+?zsh: read-only variable: foo
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS <<<'
+  readonly foo
+  foo=bar set output
+  echo output'
+1:failed assignment on posix special, POSIX_BUILTINS
+?zsh: read-only variable: foo
+
+  $ZTST_testdir/../Src/zsh -f -o POSIX_BUILTINS <<<'
+  readonly foo
+  foo=bar echo output
+  echo output'
+0:failed assignment on non-posix-special, POSIX_BUILTINS
+>output
+?zsh: read-only variable: foo

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/