[git commit] hush: make read builtin interruptible.

Denys Vlasenko vda.linux at googlemail.com
Sun May 8 19:23:43 UTC 2011


commit: http://git.busybox.net/busybox/commit/?id=80542bad2f1df9d99b579c9eeb3c2675c14c72c0
branch: http://git.busybox.net/busybox/commit/?id=refs/heads/master

function                                             old     new   delta
builtin_read                                         185     471    +286
check_and_run_traps                                  200     262     +62
nonblock_immune_read                                  73     119     +46
sigismember                                            -      44     +44
record_signal                                          -      21     +21
sigisemptyset                                          -      16     +16
...
------------------------------------------------------------------------------
(add/remove: 5/0 grow/shrink: 7/5 up/down: 483/-46)           Total: 437 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 include/libbb.h      |    2 +-
 libbb/read_printf.c  |    9 +++--
 shell/ash.c          |    6 ++--
 shell/hush.c         |   98 ++++++++++++++++++++++++++++++++++++++++++++++++-
 shell/shell_common.c |   17 ++++++++-
 5 files changed, 120 insertions(+), 12 deletions(-)

diff --git a/include/libbb.h b/include/libbb.h
index 4ea94e7..56dfa61 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -672,7 +672,7 @@ void* xrealloc_vector_helper(void *vector, unsigned sizeof_and_shift, int idx) F
 
 
 extern ssize_t safe_read(int fd, void *buf, size_t count) FAST_FUNC;
-extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count) FAST_FUNC;
+extern ssize_t nonblock_immune_read(int fd, void *buf, size_t count, int loop_on_EINTR) FAST_FUNC;
 // NB: will return short read on error, not -1,
 // if some data was read before error occurred
 extern ssize_t full_read(int fd, void *buf, size_t count) FAST_FUNC;
diff --git a/libbb/read_printf.c b/libbb/read_printf.c
index 0e6fbf6..192f83d 100644
--- a/libbb/read_printf.c
+++ b/libbb/read_printf.c
@@ -55,19 +55,20 @@
  * which detects EAGAIN and uses poll() to wait on the fd.
  * Thankfully, poll() doesn't care about O_NONBLOCK flag.
  */
-ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count)
+ssize_t FAST_FUNC nonblock_immune_read(int fd, void *buf, size_t count, int loop_on_EINTR)
 {
 	struct pollfd pfd[1];
 	ssize_t n;
 
 	while (1) {
-		n = safe_read(fd, buf, count);
+		n = loop_on_EINTR ? safe_read(fd, buf, count) : read(fd, buf, count);
 		if (n >= 0 || errno != EAGAIN)
 			return n;
 		/* fd is in O_NONBLOCK mode. Wait using poll and repeat */
 		pfd[0].fd = fd;
 		pfd[0].events = POLLIN;
-		safe_poll(pfd, 1, -1); /* note: this pulls in printf */
+		/* note: safe_poll pulls in printf */
+		loop_on_EINTR ? safe_poll(pfd, 1, -1) : poll(pfd, 1, -1);
 	}
 }
 
@@ -90,7 +91,7 @@ char* FAST_FUNC xmalloc_reads(int fd, size_t *maxsz_p)
 			p = buf + sz;
 			sz += 128;
 		}
-		if (nonblock_immune_read(fd, p, 1) != 1) {
+		if (nonblock_immune_read(fd, p, 1, /*loop_on_EINTR:*/ 1) != 1) {
 			/* EOF/error */
 			if (p == buf) { /* we read nothing */
 				free(buf);
diff --git a/shell/ash.c b/shell/ash.c
index b1b11bd..d48cd01 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -5918,7 +5918,7 @@ expbackq(union node *cmd, int quoted, int quotes)
  read:
 		if (in.fd < 0)
 			break;
-		i = nonblock_immune_read(in.fd, buf, sizeof(buf));
+		i = nonblock_immune_read(in.fd, buf, sizeof(buf), /*loop_on_EINTR:*/ 1);
 		TRACE(("expbackq: read returns %d\n", i));
 		if (i <= 0)
 			break;
@@ -9617,7 +9617,7 @@ preadfd(void)
 #if ENABLE_FEATURE_EDITING
  retry:
 	if (!iflag || g_parsefile->pf_fd != STDIN_FILENO)
-		nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1);
+		nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1, /*loop_on_EINTR:*/ 1);
 	else {
 		int timeout = -1;
 # if ENABLE_ASH_IDLE_TIMEOUT
@@ -9663,7 +9663,7 @@ preadfd(void)
 		}
 	}
 #else
-	nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1);
+	nr = nonblock_immune_read(g_parsefile->pf_fd, buf, IBUFSIZ - 1, /*loop_on_EINTR:*/ 1);
 #endif
 
 #if 0 /* disabled: nonblock_immune_read() handles this problem */
diff --git a/shell/hush.c b/shell/hush.c
index bcd4584..0b17b22 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -795,8 +795,15 @@ struct globals {
 	/* which signals have non-DFL handler (even with no traps set)? */
 	unsigned non_DFL_mask;
 	char **traps; /* char *traps[NSIG] */
-	sigset_t blocked_set;
+	/* Signal mask on the entry to the (top-level) shell. Never modified. */
 	sigset_t inherited_set;
+	/* Starts equal to inherited_set,
+	 * but shell-special signals are added and SIGCHLD is removed.
+	 * When a trap is set/cleared, signal is added to/removed from it:
+	 */
+	sigset_t blocked_set;
+	/* Used by read() */
+	sigset_t detected_set;
 #if HUSH_DEBUG
 	unsigned long memleak_value;
 	int debug_indent;
@@ -1476,6 +1483,17 @@ static int check_and_run_traps(int sig)
 		goto got_sig;
 
 	while (1) {
+		if (!sigisemptyset(&G.detected_set)) {
+			sig = 0;
+			do {
+				sig++;
+				if (sigismember(&G.detected_set, sig)) {
+					sigdelset(&G.detected_set, sig);
+					goto got_sig;
+				}
+			} while (sig < NSIG);
+		}
+
 		sig = sigtimedwait(&G.blocked_set, NULL, &zero_timespec);
 		if (sig <= 0)
 			break;
@@ -8484,6 +8502,32 @@ static int FAST_FUNC builtin_pwd(char **argv UNUSED_PARAM)
 	return EXIT_SUCCESS;
 }
 
+/* Interruptibility of read builtin in bash
+ * (tested on bash-4.2.8 by sending signals (not by ^C)):
+ *
+ * Empty trap makes read ignore corresponding signal, for any signal.
+ *
+ * SIGINT:
+ * - terminates non-interactive shell;
+ * - interrupts read in interactive shell;
+ * if it has non-empty trap:
+ * - executes trap and returns to command prompt in interactive shell;
+ * - executes trap and returns to read in non-interactive shell;
+ * SIGTERM:
+ * - is ignored (does not interrupt) read in interactive shell;
+ * - terminates non-interactive shell;
+ * if it has non-empty trap:
+ * - executes trap and returns to read;
+ * SIGHUP:
+ * - terminates shell (regardless of interactivity);
+ * if it has non-empty trap:
+ * - executes trap and returns to read;
+ */
+/* helper */
+static void record_signal(int sig)
+{
+	sigaddset(&G.detected_set, sig);
+}
 static int FAST_FUNC builtin_read(char **argv)
 {
 	const char *r;
@@ -8491,7 +8535,9 @@ static int FAST_FUNC builtin_read(char **argv)
 	char *opt_p = NULL;
 	char *opt_t = NULL;
 	char *opt_u = NULL;
+	const char *ifs;
 	int read_flags;
+	sigset_t saved_blkd_set;
 
 	/* "!": do not abort on errors.
 	 * Option string must start with "sr" to match BUILTIN_READ_xxx
@@ -8500,10 +8546,47 @@ static int FAST_FUNC builtin_read(char **argv)
 	if (read_flags == (uint32_t)-1)
 		return EXIT_FAILURE;
 	argv += optind;
+	ifs = get_local_var_value("IFS"); /* can be NULL */
+
+ again:
+	/* We need to temporarily unblock and record signals around read */
+
+	saved_blkd_set = G.blocked_set;
+	{
+		unsigned sig;
+		struct sigaction sa, old_sa;
+
+		memset(&sa, 0, sizeof(sa));
+		sigfillset(&sa.sa_mask);
+		/*sa.sa_flags = 0;*/
+		sa.sa_handler = record_signal;
+
+		sig = 0;
+		do {
+			sig++;
+			if (sigismember(&G.blocked_set, sig)) {
+				char *sig_trap = (G.traps && G.traps[sig]) ? G.traps[sig] : NULL;
+				/* If has a nonempty trap... */
+				if ((sig_trap && sig_trap[0])
+				/* ...or has no trap and is SIGINT or SIGHUP */
+				 || (!sig_trap && (sig == SIGINT || sig == SIGHUP))
+				) {
+					sigaction(sig, &sa, &old_sa);
+					if (old_sa.sa_handler == SIG_IGN) /* oops... restore back to IGN! */
+						sigaction_set(sig, &old_sa);
+					else
+						sigdelset(&G.blocked_set, sig);
+				}
+			}
+		} while (sig < NSIG-1);
+	}
+
+	if (memcmp(&saved_blkd_set, &G.blocked_set, sizeof(saved_blkd_set)) != 0)
+		sigprocmask_set(&G.blocked_set);
 
 	r = shell_builtin_read(set_local_var_from_halves,
 		argv,
-		get_local_var_value("IFS"), /* can be NULL */
+		ifs,
 		read_flags,
 		opt_n,
 		opt_p,
@@ -8511,6 +8594,17 @@ static int FAST_FUNC builtin_read(char **argv)
 		opt_u
 	);
 
+	if (memcmp(&saved_blkd_set, &G.blocked_set, sizeof(saved_blkd_set)) != 0) {
+		G.blocked_set = saved_blkd_set;
+		sigprocmask_set(&G.blocked_set);
+	}
+
+	if ((uintptr_t)r == 1 && errno == EINTR) {
+		unsigned sig = check_and_run_traps(0);
+		if (sig && sig != SIGINT)
+			goto again;
+	}
+
 	if ((uintptr_t)r > 1) {
 		bb_error_msg("%s", r);
 		r = (char*)(uintptr_t)1;
diff --git a/shell/shell_common.c b/shell/shell_common.c
index 86a6493..a5c455c 100644
--- a/shell/shell_common.c
+++ b/shell/shell_common.c
@@ -36,6 +36,10 @@ int FAST_FUNC is_well_formed_var_name(const char *s, char terminator)
 
 /* read builtin */
 
+/* Needs to be interruptible: shell mush handle traps and shell-special signals
+ * while inside read. To implement this, be sure to not loop on EINTR
+ * and return errno == EINTR reliably.
+ */
 //TODO: use more efficient setvar() which takes a pointer to malloced "VAR=VAL"
 //string. hush naturally has it, and ash has setvareq().
 //Here we can simply store "VAR=" at buffer start and store read data directly
@@ -51,6 +55,7 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	const char *opt_u
 )
 {
+	unsigned err;
 	unsigned end_ms; /* -t TIMEOUT */
 	int fd; /* -u FD */
 	int nchars; /* -n NUM */
@@ -62,6 +67,8 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	int startword;
 	smallint backslash;
 
+	errno = err = 0;
+
 	pp = argv;
 	while (*pp) {
 		if (!is_well_formed_var_name(*pp, '\0')) {
@@ -153,6 +160,8 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	do {
 		char c;
 
+		errno = 0;
+
 		if (end_ms) {
 			int timeout;
 			struct pollfd pfd[1];
@@ -161,8 +170,9 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 			pfd[0].events = POLLIN;
 			timeout = end_ms - (unsigned)monotonic_ms();
 			if (timeout <= 0 /* already late? */
-			 || safe_poll(pfd, 1, timeout) != 1 /* no? wait... */
+			 || poll(pfd, 1, timeout) != 1 /* no? wait... */
 			) { /* timed out! */
+				err = errno;
 				retval = (const char *)(uintptr_t)1;
 				goto ret;
 			}
@@ -170,7 +180,8 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 
 		if ((bufpos & 0xff) == 0)
 			buffer = xrealloc(buffer, bufpos + 0x100);
-		if (nonblock_immune_read(fd, &buffer[bufpos], 1) != 1) {
+		if (nonblock_immune_read(fd, &buffer[bufpos], 1, /*loop_on_EINTR:*/ 0) != 1) {
+			err = errno;
 			retval = (const char *)(uintptr_t)1;
 			break;
 		}
@@ -240,6 +251,8 @@ shell_builtin_read(void FAST_FUNC (*setvar)(const char *name, const char *val),
 	free(buffer);
 	if (read_flags & BUILTIN_READ_SILENT)
 		tcsetattr(fd, TCSANOW, &old_tty);
+
+	errno = err;
 	return retval;
 }
 
-- 
1.7.3.4



More information about the busybox-cvs mailing list