Fix pty handling on non-AIX, non-HP-UX systems.

7 files changed, 258 insertions, 103 deletions

diff --git a/ChangeLog b/ChangeLog
index 361c3488..6f9f2867 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2018-01-04         Arnold D. Robbins     <arnold@skeeve.com>
+
+	Refactor handling of slave pty. On AIX and HP-UX open
+	slave in the child. Otherwise open slave in the parent
+	before forking (restoring code mostly from 4.1.3). Thanks
+	to Andrew Schorr for the bug report.
+
+	* io.c (fork_and_open_slave_pty): New routine. Two versions.
+	(set_slave_pty_attributes): New routine. Common code used by
+	both versions of fork_and_open_slave_pty.
+	(push_pty_line_disciplines): New routine. Common code used by
+	both versions of fork_and_open_slave_pty.
+	(two_way_open): Call fork_and_open_slave_pty instead of
+	doing it inline.
+
 2018-01-03         Arnold D. Robbins     <arnold@skeeve.com>
 
 	* main.c (UPDATE_YEAR): Move to 2018. Revise copyright year.
diff --git a/io.c b/io.c
index 7314d8e8..994eb4a5 100644
--- a/io.c
+++ b/io.c
@@ -1892,6 +1892,206 @@ strictopen:
 	return openfd;
 }
 
+/* push_pty_line_disciplines --- push line disciplines if we work that way */
+
+// Factors out common code for the two versions of fork_and_open_slave_pty().
+
+static void
+push_pty_line_disciplines(int slave)
+{
+#ifdef I_PUSH
+	/*
+	 * Push the necessary modules onto the slave to
+	 * get terminal semantics.  Check that they aren't
+	 * already there to avoid hangs on said "limited" systems.
+	 */
+#ifdef I_FIND
+	if (ioctl(slave, I_FIND, "ptem") == 0)
+#endif
+		ioctl(slave, I_PUSH, "ptem");
+#ifdef I_FIND
+	if (ioctl(slave, I_FIND, "ldterm") == 0)
+#endif
+		ioctl(slave, I_PUSH, "ldterm");
+#endif
+}
+
+/* set_slave_pty_attributes --- set terminal attributes for slave pty */
+
+// Factors out common code for the two versions of fork_and_open_slave_pty().
+
+static void
+set_slave_pty_attributes(int slave)
+{
+	struct termios st;
+
+	tcgetattr(slave, & st);
+	st.c_iflag &= ~(ISTRIP | IGNCR | INLCR | IXOFF);
+	st.c_iflag |= (ICRNL | IGNPAR | BRKINT | IXON);
+	st.c_oflag &= ~OPOST;
+	st.c_cflag &= ~CSIZE;
+	st.c_cflag |= CREAD | CS8 | CLOCAL;
+	st.c_lflag &= ~(ECHO | ECHOE | ECHOK | NOFLSH | TOSTOP);
+	st.c_lflag |= ISIG;
+
+	/* Set some control codes to default values */
+#ifdef VINTR
+	st.c_cc[VINTR] = '\003';        /* ^c */
+#endif
+#ifdef VQUIT
+	st.c_cc[VQUIT] = '\034';        /* ^| */
+#endif
+#ifdef VERASE
+	st.c_cc[VERASE] = '\177';       /* ^? */
+#endif
+#ifdef VKILL
+	st.c_cc[VKILL] = '\025';        /* ^u */
+#endif
+#ifdef VEOF
+	st.c_cc[VEOF] = '\004'; /* ^d */
+#endif
+	tcsetattr(slave, TCSANOW, & st);
+}
+
+
+/* fork_and_open_slave_pty --- handle forking the child and slave pty setup */
+
+/*
+ * January, 2018:
+ * This is messy. AIX and HP-UX require that the slave pty be opened and
+ * set up in the child.  Everything else wants it to be done in the parent,
+ * before the fork.  Thus we have two different versions of the routine that
+ * do the same thing, but in different orders.  This is not pretty, but it
+ * seems to be the simplest thing to do.
+ */
+
+#if defined _AIX || defined __hpux
+static bool
+fork_and_open_slave_pty(const char *slavenam, int master, const char *command, pid_t *pid)
+{
+	int slave;
+	int save_errno;
+
+	/*
+	 * We specifically open the slave only in the child. This allows
+	 * certain, er, "limited" systems to work.  The open is specifically
+	 * without O_NOCTTY in order to make the slave become the controlling
+	 * terminal.
+	 */
+
+	switch (*pid = fork()) {
+	case 0:
+		/* Child process */
+		setsid();
+
+		if ((slave = open(slavenam, O_RDWR)) < 0) {
+			close(master);
+			fatal(_("could not open `%s', mode `%s'"),
+				slavenam, "r+");
+		}
+
+		push_pty_line_disciplines(slave);
+		set_slave_pty_attributes(slave);
+
+		if (close(master) == -1)
+			fatal(_("close of master pty failed (%s)"), strerror(errno));
+		if (close(1) == -1)
+			fatal(_("close of stdout in child failed (%s)"),
+				strerror(errno));
+		if (dup(slave) != 1)
+			fatal(_("moving slave pty to stdout in child failed (dup: %s)"), strerror(errno));
+		if (close(0) == -1)
+			fatal(_("close of stdin in child failed (%s)"),
+				strerror(errno));
+		if (dup(slave) != 0)
+			fatal(_("moving slave pty to stdin in child failed (dup: %s)"), strerror(errno));
+		if (close(slave))
+			fatal(_("close of slave pty failed (%s)"), strerror(errno));
+
+		/* stderr does NOT get dup'ed onto child's stdout */
+
+		set_sigpipe_to_default();
+
+		execl("/bin/sh", "sh", "-c", command, NULL);
+		_exit(errno == ENOENT ? 127 : 126);
+
+	case -1:
+		save_errno = errno;
+		close(master);
+		errno = save_errno;
+		return false;
+
+	default:
+		return true;
+	}
+}
+#else
+static bool
+fork_and_open_slave_pty(const char *slavenam, int master, const char *command, pid_t *pid)
+{
+	int slave;
+	int save_errno;
+
+	if ((slave = open(slavenam, O_RDWR)) < 0) {
+		close(master);
+		fatal(_("could not open `%s', mode `%s'"),
+			slavenam, "r+");
+	}
+
+	push_pty_line_disciplines(slave);
+	set_slave_pty_attributes(slave);
+
+	switch (*pid = fork()) {
+	case 0:
+		/* Child process */
+		setsid();
+
+#ifdef TIOCSCTTY
+		ioctl(slave, TIOCSCTTY, 0);
+#endif
+
+		if (close(master) == -1)
+			fatal(_("close of master pty failed (%s)"), strerror(errno));
+		if (close(1) == -1)
+			fatal(_("close of stdout in child failed (%s)"),
+				strerror(errno));
+		if (dup(slave) != 1)
+			fatal(_("moving slave pty to stdout in child failed (dup: %s)"), strerror(errno));
+		if (close(0) == -1)
+			fatal(_("close of stdin in child failed (%s)"),
+				strerror(errno));
+		if (dup(slave) != 0)
+			fatal(_("moving slave pty to stdin in child failed (dup: %s)"), strerror(errno));
+		if (close(slave))
+			fatal(_("close of slave pty failed (%s)"), strerror(errno));
+
+		/* stderr does NOT get dup'ed onto child's stdout */
+
+		signal(SIGPIPE, SIG_DFL);
+
+		execl("/bin/sh", "sh", "-c", command, NULL);
+		_exit(errno == ENOENT ? 127 : 126);
+
+	case -1:
+		save_errno = errno;
+		close(master);
+		close(slave);
+		errno = save_errno;
+		return false;
+
+	}
+
+	/* parent */
+	if (close(slave) != 0) {
+		close(master);
+		(void) kill(*pid, SIGKILL);
+		fatal(_("close of slave pty failed (%s)"), strerror(errno));
+	}
+
+	return true;
+}
+#endif
+
 /* two_way_open --- open a two way communications channel */
 
 static int
@@ -1955,11 +2155,8 @@ two_way_open(const char *str, struct redirect *rp, int extfd)
 		char slavenam[32];
 		char c;
 		int master, dup_master;
-		int slave;
-		int save_errno;
 		pid_t pid;
 		struct stat statb;
-		struct termios st;
 		/* Use array of chars to avoid ASCII / EBCDIC issues */
 		static char pty_chars[] = "pqrstuvwxyzabcdefghijklmno";
 		int i;
@@ -2046,104 +2243,9 @@ two_way_open(const char *str, struct redirect *rp, int extfd)
 
 	got_the_pty:
 
-		/*
-		 * We specifically open the slave only in the child. This allows
-		 * certain, er, "limited" systems to work.  The open is specifically
-		 * without O_NOCTTY in order to make the slave become the controlling
-		 * terminal.
-		 */
-
-		switch (pid = fork()) {
-		case 0:
-			/* Child process */
-			setsid();
-
-			if ((slave = open(slavenam, O_RDWR)) < 0) {
-				close(master);
-				fatal(_("could not open `%s', mode `%s'"),
-					slavenam, "r+");
-			}
-
-#ifdef I_PUSH
-			/*
-			 * Push the necessary modules onto the slave to
-			 * get terminal semantics.  Check that they aren't
-			 * already there to avoid hangs on said "limited" systems.
-			 */
-#ifdef I_FIND
-			if (ioctl(slave, I_FIND, "ptem") == 0)
-#endif
-				ioctl(slave, I_PUSH, "ptem");
-#ifdef I_FIND
-			if (ioctl(slave, I_FIND, "ldterm") == 0)
-#endif
-				ioctl(slave, I_PUSH, "ldterm");
-#endif
-			tcgetattr(slave, & st);
-
-			st.c_iflag &= ~(ISTRIP | IGNCR | INLCR | IXOFF);
-			st.c_iflag |= (ICRNL | IGNPAR | BRKINT | IXON);
-			st.c_oflag &= ~OPOST;
-			st.c_cflag &= ~CSIZE;
-			st.c_cflag |= CREAD | CS8 | CLOCAL;
-			st.c_lflag &= ~(ECHO | ECHOE | ECHOK | NOFLSH | TOSTOP);
-			st.c_lflag |= ISIG;
-
-			/* Set some control codes to default values */
-#ifdef VINTR
-			st.c_cc[VINTR] = '\003';        /* ^c */
-#endif
-#ifdef VQUIT
-			st.c_cc[VQUIT] = '\034';        /* ^| */
-#endif
-#ifdef VERASE
-			st.c_cc[VERASE] = '\177';       /* ^? */
-#endif
-#ifdef VKILL
-			st.c_cc[VKILL] = '\025';        /* ^u */
-#endif
-#ifdef VEOF
-			st.c_cc[VEOF] = '\004'; /* ^d */
-#endif
-
-#ifdef TIOCSCTTY
-			/*
-			 * This may not necessary anymore given that we
-			 * open the slave in the child, but it doesn't hurt.
-			 */
-			ioctl(slave, TIOCSCTTY, 0);
-#endif
-			tcsetattr(slave, TCSANOW, & st);
-
-			if (close(master) == -1)
-				fatal(_("close of master pty failed (%s)"), strerror(errno));
-			if (close(1) == -1)
-				fatal(_("close of stdout in child failed (%s)"),
-					strerror(errno));
-			if (dup(slave) != 1)
-				fatal(_("moving slave pty to stdout in child failed (dup: %s)"), strerror(errno));
-			if (close(0) == -1)
-				fatal(_("close of stdin in child failed (%s)"),
-					strerror(errno));
-			if (dup(slave) != 0)
-				fatal(_("moving slave pty to stdin in child failed (dup: %s)"), strerror(errno));
-			if (close(slave))
-				fatal(_("close of slave pty failed (%s)"), strerror(errno));
-
-			/* stderr does NOT get dup'ed onto child's stdout */
-
-			set_sigpipe_to_default();
-
-			execl("/bin/sh", "sh", "-c", str, NULL);
-			_exit(errno == ENOENT ? 127 : 126);
-
-		case -1:
-			save_errno = errno;
-			close(master);
-			errno = save_errno;
-			return false;
-
-		}
+		/* this is the parent */
+		if (! fork_and_open_slave_pty(slavenam, master, str, & pid))
+			fatal(_("could not create child process or open pty"));
 
 		rp->pid = pid;
 		rp->iop = iop_alloc(master, str, 0);
diff --git a/test/ChangeLog b/test/ChangeLog
index c64dc5bc..5799b03d 100644
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,3 +1,10 @@
+2018-01-04         Arnold D. Robbins     <arnold@skeeve.com>
+
+	Thanks to Andrew Schorr for the basics of this test.
+
+	* Makefile.am (pty2): New test. 
+	* pty2.awk, pty2.ok: New files.
+
 2018-01-02         Arnold D. Robbins     <arnold@skeeve.com>
 
 	Thanks to Nethox <nethox@gmail.com> for this test.
diff --git a/test/Makefile.am b/test/Makefile.am
index 1ec63a20..3c9f2b0c 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -880,6 +880,8 @@ EXTRA_DIST = \
 	prtoeval.ok \
 	pty1.awk \
 	pty1.ok \
+	pty2.awk \
+	pty2.ok \
 	rand-mpfr.ok \
 	rand.awk \
 	rand.ok \
@@ -1269,7 +1271,7 @@ GAWK_EXT_TESTS = \
 	nastyparm negtime next nondec nondec2 nonfatal1 nonfatal2 nonfatal3 \
 	patsplit posix printfbad1 printfbad2 printfbad3 printfbad4 printhuge \
 	procinfs profile0 profile1 profile2 profile3 profile4 profile5 profile6 \
-	profile7 profile8 profile9 profile10 pty1 \
+	profile7 profile8 profile9 profile10 pty1 pty2 \
 	rebuf regnul1 regnul2 regx8bit reginttrad reint reint2 rsgetline rsglstdin \
 	rsstart1 rsstart2 rsstart3 rstest6 \
 	shadow shadowbuiltin sortfor sortfor2 sortu sourcesplit split_after_fpat \
@@ -2404,6 +2406,14 @@ pty1:
 	$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ ;; \
 	esac
 
+pty2:
+	@echo $@
+	@-case `uname` in \
+	*[Oo][Ss]/390*) : ;; \
+	*) AWKPATH="$(srcdir)" $(AWK) -f $@.awk | od -c  >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ ; \
+	$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ ;; \
+	esac
+
 rscompat:
 	@echo $@
 	@AWKPATH="$(srcdir)" $(AWK) --traditional -f $@.awk "$(srcdir)/$@.in" >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@
diff --git a/test/Makefile.in b/test/Makefile.in
index 9775fd5b..482040a7 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -1138,6 +1138,8 @@ EXTRA_DIST = \
 	prtoeval.ok \
 	pty1.awk \
 	pty1.ok \
+	pty2.awk \
+	pty2.ok \
 	rand-mpfr.ok \
 	rand.awk \
 	rand.ok \
@@ -1526,7 +1528,7 @@ GAWK_EXT_TESTS = \
 	nastyparm negtime next nondec nondec2 nonfatal1 nonfatal2 nonfatal3 \
 	patsplit posix printfbad1 printfbad2 printfbad3 printfbad4 printhuge \
 	procinfs profile0 profile1 profile2 profile3 profile4 profile5 profile6 \
-	profile7 profile8 profile9 profile10 pty1 \
+	profile7 profile8 profile9 profile10 pty1 pty2 \
 	rebuf regnul1 regnul2 regx8bit reginttrad reint reint2 rsgetline rsglstdin \
 	rsstart1 rsstart2 rsstart3 rstest6 \
 	shadow shadowbuiltin sortfor sortfor2 sortu sourcesplit split_after_fpat \
@@ -2842,6 +2844,14 @@ pty1:
 	$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ ;; \
 	esac
 
+pty2:
+	@echo $@
+	@-case `uname` in \
+	*[Oo][Ss]/390*) : ;; \
+	*) AWKPATH="$(srcdir)" $(AWK) -f $@.awk | od -c  >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ ; \
+	$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ ;; \
+	esac
+
 rscompat:
 	@echo $@
 	@AWKPATH="$(srcdir)" $(AWK) --traditional -f $@.awk "$(srcdir)/$@.in" >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@
diff --git a/test/pty2.awk b/test/pty2.awk
new file mode 100644
index 00000000..c2eedbd0
--- /dev/null
+++ b/test/pty2.awk
@@ -0,0 +1,9 @@
+BEGIN {
+   cmd = "tr '[A-Z]' '[a-z]' 2> /dev/null"
+   PROCINFO[cmd, "pty"] = 1
+   input = "ABCD"
+   print input |& cmd
+   cmd |& getline x
+   print x
+#   close(cmd)
+}
diff --git a/test/pty2.ok b/test/pty2.ok
new file mode 100644
index 00000000..fb0f6221
--- /dev/null
+++ b/test/pty2.ok
@@ -0,0 +1,2 @@
+0000000   a   b   c   d  \n
+0000005