zsh-workers
 help / color / mirror / code / Atom feed
From: Bart Schaefer <schaefer@brasslantern.com>
To: Dipak Gaigole <dipakgaigole@gmail.com>, zsh-workers@zsh.org
Subject: Re: zsh behavior when fork() failed
Date: Sun, 26 Feb 2012 11:52:20 -0800	[thread overview]
Message-ID: <120226115220.ZM17815@torch.brasslantern.com> (raw)
In-Reply-To: <CADs2-=RNMAHz=fQOWjVm25XRL4xaGLzTtA7yL-FCQPrHdV5jpg@mail.gmail.com>

On Feb 25, 10:03pm, Dipak Gaigole wrote:
} Subject: Re: zsh behavior when fork() failed
}
} > Section 2.8.1 lists command execution failures where a non-interactive
} > shell "shall exit", but fork failure is not among them.
} 
} Got it.

That doesn't mean I didn't miss something.  (I was kind of in a hurry
on Friday ...)

This also raises the question of whether a non-interactive shell should
exit on a botched "exec" (see zsh-workers/30111 and subsequent thread,
and comment on line 3364 excerpted below).

} But I don't want to add a check of $? after each command in
} the scripts specially in cases where scripts are 1000+ lines.

Would "set -e" (setopt err_exit) do what you need?

} Is there any zsh option to make sure script exit when fork fails? Else
} can I patch the zsh code to make script exit on fork failure?

It's entirely possible that "return;" on lines 2818 and 2824 in exec.c:

2814         if (pipe(synch) < 0) {
2815             zerr("pipe failed: %e", errno);
2816             if (oautocont >= 0)
2817                 opts[AUTOCONTINUE] = oautocont;
2818             return;
2819         } else if ((pid = zfork(&bgtime)) == -1) {
2820             close(synch[0]);
2821             close(synch[1]);
2822             if (oautocont >= 0)
2823                 opts[AUTOCONTINUE] = oautocont;
2824             return;
2825         }

should instead behave the way POSIXBUILTINS does:

3362         /*
3363          * For POSIX-compatible behaviour with special
3364          * builtins (including exec which we don't usually
3365          * classify as a builtin) we treat all errors as fatal.
3366          * The "command" builtin is not special so resets this behaviour.
3367          */
3368         if (redir_err || errflag) {
3369             if (!isset(INTERACTIVE)) {
3370                 if (forked)
3371                     _exit(1);
3372                 else
3373                     exit(1);
3374             }
3375             errflag = 1;
3376         }

There are a couple of other cases (e.g., around line 1680) where's also
possible the error should be considered unrecoverable.

That would make a full patch look something like the following (which
still exits with 1, rather than with 128 like bash, but does exit).
The oautocont stuff goes away because it's handled below the "fatal"
label (outside the visible diff context).  The second hunk still might
not cause a proper exit, it's in execpline2() from which it's less clear
to me how to reach the "fatal" condition but it's pretty obvious that
it shouldn't just be falling through in those error states.


Index: Src/exec.c
--- ../zsh-forge/current/Src/exec.c	2012-02-12 13:31:49.000000000 -0800
+++ Src/exec.c	2012-02-26 11:47:48.000000000 -0800
@@ -1617,9 +1617,8 @@
 		 (list_pipe || (pline_level && !(jn->stat & STAT_SUBJOB)))))
 		deletejob(jn, 0);
 	    thisjob = pj;
-
 	}
-	if (slflags & WC_SUBLIST_NOT)
+	if ((slflags & WC_SUBLIST_NOT) && !errflag)
 	    lastval = !lastval;
     }
     if (!pline_level)
@@ -1679,9 +1678,13 @@
 
 	    if (pipe(synch) < 0) {
 		zerr("pipe failed: %e", errno);
+		lastval = errflag = 1;
+		return;
 	    } else if ((pid = zfork(&bgtime)) == -1) {
 		close(synch[0]);
 		close(synch[1]);
+		lastval = errflag = 1;
+		return;
 	    } else if (pid) {
 		char dummy, *text;
 
@@ -2490,7 +2493,7 @@
 		    if (!firstnode(args)) {
 			zerr("exec requires a command to execute");
 			errflag = lastval = 1;
-			return;
+			goto fatal;
 		    }
 		    uremnode(args, firstnode(args));
 		    if (!strcmp(next, "--"))
@@ -2507,12 +2510,12 @@
 				if (!firstnode(args)) {
 				    zerr("exec requires a command to execute");
 				    errflag = lastval = 1;
-				    return;
+				    goto fatal;
 				}
 				if (!nextnode(firstnode(args))) {
 				    zerr("exec flag -a requires a parameter");
 				    errflag = lastval = 1;
-				    return;
+				    goto fatal;
 				}
 				exec_argv0 = (char *)
 				    getdata(nextnode(firstnode(args)));
@@ -2813,15 +2816,12 @@
 
 	if (pipe(synch) < 0) {
 	    zerr("pipe failed: %e", errno);
-	    if (oautocont >= 0)
-		opts[AUTOCONTINUE] = oautocont;
-	    return;
+	    goto fatal;
 	} else if ((pid = zfork(&bgtime)) == -1) {
 	    close(synch[0]);
 	    close(synch[1]);
-	    if (oautocont >= 0)
-		opts[AUTOCONTINUE] = oautocont;
-	    return;
+	    lastval = errflag = 1;
+	    goto fatal;
 	}
 	if (pid) {
 
@@ -3365,6 +3365,7 @@
 	 * classify as a builtin) we treat all errors as fatal.
 	 * The "command" builtin is not special so resets this behaviour.
 	 */
+    fatal:
 	if (redir_err || errflag) {
 	    if (!isset(INTERACTIVE)) {
 		if (forked)


  reply	other threads:[~2012-02-26 20:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-23 10:40 Dipak Gaigole
2012-02-23 16:14 ` Bart Schaefer
2012-02-24 11:08   ` Dipak Gaigole
2012-02-24 18:05     ` Bart Schaefer
2012-02-25 16:33       ` Dipak Gaigole
2012-02-26 19:52         ` Bart Schaefer [this message]
2012-02-26 21:57           ` Bart Schaefer
2012-02-29 12:57           ` Dipak Gaigole
2012-02-29 19:06             ` Bart Schaefer
2012-03-06  9:01               ` Dipak Gaigole

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=120226115220.ZM17815@torch.brasslantern.com \
    --to=schaefer@brasslantern.com \
    --cc=dipakgaigole@gmail.com \
    --cc=zsh-workers@zsh.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

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).