← Commit list

80542bad2

Denys Vlasenko

2011-05-08

FAILED Duration: 222s

Subject

hush: make read builtin interruptible.

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

include/libbb.h +1 -1 libbb/read_printf.c +5 -4 shell/ash.c +3 -3 shell/hush.c +96 -2 shell/shell_common.c +15 -2

include/libbb.h+1 -1

@@ -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;

libbb/read_printf.c+5 -4

@@ -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);

shell/ash.c+3 -3

@@ -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 */

shell/hush.c+96 -2

@@ -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;

shell/shell_common.c+15 -2

@@ -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;
	}