[hackers] [st][PATCH] remove secondary call to select

From: Jeremy Bobbin <jer_AT_jer.cx>
Date: Thu, 27 Jun 2024 18:51:28 -0700

because underlying programs can request info from the tty, there was a
potential recursion in ttyread:
        1. ttyread
        2. tty program wants window size
        3. ttywrite the window size

this assumes the ttyfd is both ready for read & write.

if, for example, the tty wants st to report the cursor position & the
write pipe is already cloggled, then this could cause a deadlock.

this was initialy addressed with 2 selects:

one in x.c which scanned for { display events & tty for reading }.
another in st.c which scanned for { read & write }-ability of the tty.

this patch, instead, buffers what we write to the TTY, and then
conditionally selects over tty readability, tty writability & display
events.

in addition to simplifying the ttywrite function, this will make fewer
calls to select & should make fewer calls to write.

---
 st.c | 92 ++++++++++++++++++++++++++++--------------------------------
 st.h |  3 ++
 x.c  |  9 +++++-
 3 files changed, 54 insertions(+), 50 deletions(-)
diff --git a/st.c b/st.c
index 57c6e96..030c294 100644
--- a/st.c
+++ b/st.c
_AT_@ -152,6 +152,12 @@ typedef struct {
 	int narg;              /* nb of args */
 } STREscape;
 
+typedef struct {
+	char *buf;             /* allocated raw string */
+	size_t siz;            /* allocation size */
+	size_t len;            /* raw string length */
+} TBuffer;
+
 static void execsh(char *, char **);
 static void stty(char **);
 static void sigchld(int);
_AT_@ -223,6 +229,7 @@ static Term term;
 static Selection sel;
 static CSIEscape csiescseq;
 static STREscape strescseq;
+static TBuffer tbuf;
 static int iofd = 1;
 static int cmdfd;
 static pid_t pid;
_AT_@ -752,6 +759,35 @@ stty(char **args)
 		perror("Couldn't call stty");
 }
 
+void
+tbufnew(size_t siz) {
+	tbuf.buf = (char *)xmalloc(siz);
+	tbuf.siz = siz;
+	tbuf.len = 0;
+}
+
+size_t
+tbuflen(void) {
+	return tbuf.len;
+}
+
+size_t
+tbufwrite(void) {
+	size_t lim = 256;
+	ssize_t r;
+
+	/*
+	 * Migrate the world to Plan 9.
+	 */
+	if ((r = write(cmdfd, &tbuf.buf[0], MIN(tbuf.len, lim))) < 0)
+		die("write error on tty: %s\n", strerror(errno));
+
+	tbuf.len -= r;
+	memmove(tbuf.buf, &tbuf.buf[r], tbuf.len);
+
+	return r;
+}
+
 int
 ttynew(const char *line, char *cmd, const char *out, char **args)
 {
_AT_@ -868,61 +904,19 @@ ttywrite(const char *s, size_t n, int may_echo)
 	}
 }
 
+
 void
 ttywriteraw(const char *s, size_t n)
 {
-	fd_set wfd, rfd;
-	ssize_t r;
-	size_t lim = 256;
-
-	/*
-	 * Remember that we are using a pty, which might be a modem line.
-	 * Writing too much will clog the line. That's why we are doing this
-	 * dance.
-	 * FIXME: Migrate the world to Plan 9.
-	 */
-	while (n > 0) {
-		FD_ZERO(&wfd);
-		FD_ZERO(&rfd);
-		FD_SET(cmdfd, &wfd);
-		FD_SET(cmdfd, &rfd);
-
-		/* Check if we can write. */
-		if (pselect(cmdfd+1, &rfd, &wfd, NULL, NULL, NULL) < 0) {
-			if (errno == EINTR)
-				continue;
-			die("select failed: %s\n", strerror(errno));
-		}
-		if (FD_ISSET(cmdfd, &wfd)) {
-			/*
-			 * Only write the bytes written by ttywrite() or the
-			 * default of 256. This seems to be a reasonable value
-			 * for a serial line. Bigger values might clog the I/O.
-			 */
-			if ((r = write(cmdfd, s, (n < lim)? n : lim)) < 0)
-				goto write_error;
-			if (r < n) {
-				/*
-				 * We weren't able to write out everything.
-				 * This means the buffer is getting full
-				 * again. Empty it.
-				 */
-				if (n < lim)
-					lim = ttyread();
-				n -= r;
-				s += r;
-			} else {
-				/* All bytes have been written. */
-				break;
-			}
-		}
-		if (FD_ISSET(cmdfd, &rfd))
-			lim = ttyread();
+	if (tbuf.siz - tbuf.len < n) {
+		tbuf.siz *= 2;
+		tbuf.buf = xrealloc(tbuf.buf, tbuf.siz);
 	}
+	memcpy(&tbuf.buf[tbuf.len], s, n);
+	tbuf.len += n;
+
 	return;
 
-write_error:
-	die("write error on tty: %s\n", strerror(errno));
 }
 
 void
diff --git a/st.h b/st.h
index fd3b0d8..2b810b8 100644
--- a/st.h
+++ b/st.h
_AT_@ -87,6 +87,9 @@ void sendbreak(const Arg *);
 void toggleprinter(const Arg *);
 
 int tattrset(int);
+void tbufnew(size_t);
+size_t tbuflen(void);
+size_t tbufwrite(void);
 void tnew(int, int);
 void tresize(int, int);
 void tsetdirtattr(int);
diff --git a/x.c b/x.c
index bd23686..f1ac1a9 100644
--- a/x.c
+++ b/x.c
_AT_@ -1922,7 +1922,7 @@ run(void)
 {
 	XEvent ev;
 	int w = win.w, h = win.h;
-	fd_set rfd;
+	fd_set rfd, wfd;
 	int xfd = XConnectionNumber(xw.dpy), ttyfd, xev, drawing;
 	struct timespec seltv, *tv, now, lastblink, trigger;
 	double timeout;
_AT_@ -1948,8 +1948,11 @@ run(void)
 
 	for (timeout = -1, drawing = 0, lastblink = (struct timespec){0};;) {
 		FD_ZERO(&rfd);
+		FD_ZERO(&wfd);
 		FD_SET(ttyfd, &rfd);
 		FD_SET(xfd, &rfd);
+		if (tbuflen() > 0)
+			FD_SET(ttyfd, &wfd);
 
 		if (XPending(xw.dpy))
 			timeout = 0;  /* existing events might not set xfd */
_AT_@ -1968,6 +1971,9 @@ run(void)
 		if (FD_ISSET(ttyfd, &rfd))
 			ttyread();
 
+		if (FD_ISSET(ttyfd, &wfd))
+			tbufwrite();
+
 		xev = 0;
 		while (XPending(xw.dpy)) {
 			xev = 1;
_AT_@ -2095,6 +2101,7 @@ run:
 	XSetLocaleModifiers("");
 	cols = MAX(cols, 1);
 	rows = MAX(rows, 1);
+	tbufnew(256);
 	tnew(cols, rows);
 	xinit(cols, rows);
 	xsetenv();
-- 
2.45.2

Received on Fri Jun 28 2024 - 03:51:28 CEST