From 4ddaef74ff8f605a31c499d77fcaaffa7a3708c0 Mon Sep 17 00:00:00 2001
From: Andre Noll <maan@tuebingen.mpg.de>
Date: Thu, 30 Sep 2021 21:46:58 +0200
Subject: [PATCH] fd: Drop fd_set parameter from read_nonblock() and friends.

This parameter is not necessary because its only purpose is to
avoid the readv(2) system call in case it would likely return EAGAIN
because we just called select(2) which reported that there is no data
to read. Since the parameter is an obstacle for the conversion of
the code base from select(2) to poll(2), get rid of it for the time
being. If needed we can add back an equivalent optimization which
checks for POLLIN after the conversion.
---
 afs.c           | 14 +++++++-------
 audiod.c        |  2 +-
 client_common.c | 15 ++++++---------
 dccp_recv.c     |  4 ++--
 fd.c            | 33 ++++++++-------------------------
 fd.h            |  7 +++----
 gui.c           | 10 +++++-----
 http_recv.c     |  4 ++--
 http_send.c     |  2 +-
 server.c        |  2 +-
 signal.c        |  6 ++----
 signal.h        |  2 +-
 stdin.c         |  4 ++--
 udp_recv.c      |  2 +-
 14 files changed, 42 insertions(+), 65 deletions(-)

diff --git a/afs.c b/afs.c
index c216b354..c2081db7 100644
--- a/afs.c
+++ b/afs.c
@@ -715,7 +715,7 @@ static int afs_signal_post_select(struct sched *s, __a_unused void *context)
 		PARA_EMERG_LOG("para_server died\n");
 		goto shutdown;
 	}
-	signum = para_next_signal(&s->rfds);
+	signum = para_next_signal();
 	if (signum == 0)
 		return 0;
 	if (signum == SIGHUP) {
@@ -862,11 +862,11 @@ static int call_callback(int fd, int query_shmid)
 	return ret;
 }
 
-static int execute_server_command(fd_set *rfds)
+static int execute_server_command(void)
 {
 	char buf[8];
 	size_t n;
-	int ret = read_nonblock(server_socket, buf, sizeof(buf) - 1, rfds, &n);
+	int ret = read_nonblock(server_socket, buf, sizeof(buf) - 1, &n);
 
 	if (ret < 0 || n == 0)
 		return ret;
@@ -877,13 +877,13 @@ static int execute_server_command(fd_set *rfds)
 }
 
 /* returns 0 if no data available, 1 else */
-static int execute_afs_command(int fd, fd_set *rfds)
+static int execute_afs_command(int fd)
 {
 	uint32_t cookie;
 	int query_shmid;
 	char buf[sizeof(cookie) + sizeof(query_shmid)];
 	size_t n;
-	int ret = read_nonblock(fd, buf, sizeof(buf), rfds, &n);
+	int ret = read_nonblock(fd, buf, sizeof(buf), &n);
 
 	if (ret < 0)
 		goto err;
@@ -927,7 +927,7 @@ static int command_post_select(struct sched *s, void *context)
 	ret = task_get_notification(ct->task);
 	if (ret < 0)
 		return ret;
-	ret = execute_server_command(&s->rfds);
+	ret = execute_server_command();
 	if (ret < 0) {
 		PARA_EMERG_LOG("%s\n", para_strerror(-ret));
 		task_notify_all(s, -ret);
@@ -935,7 +935,7 @@ static int command_post_select(struct sched *s, void *context)
 	}
 	/* Check the list of connected clients. */
 	list_for_each_entry_safe(client, tmp, &afs_client_list, node) {
-		ret = execute_afs_command(client->fd, &s->rfds);
+		ret = execute_afs_command(client->fd);
 		if (ret == 0) { /* prevent bogus connection flooding */
 			struct timeval diff;
 			tv_diff(now, &client->connect_time, &diff);
diff --git a/audiod.c b/audiod.c
index 8e1da6e8..482cce37 100644
--- a/audiod.c
+++ b/audiod.c
@@ -1063,7 +1063,7 @@ static int signal_post_select(struct sched *s, void *context)
 	ret = task_get_notification(st->task);
 	if (ret < 0)
 		return ret;
-	signum = para_next_signal(&s->rfds);
+	signum = para_next_signal();
 	switch (signum) {
 	case SIGINT:
 	case SIGTERM:
diff --git a/client_common.c b/client_common.c
index f8ee80c9..499b6f95 100644
--- a/client_common.c
+++ b/client_common.c
@@ -125,8 +125,7 @@ static int send_sb(struct client_task *ct, int channel, void *buf, size_t numbyt
 	return 0;
 }
 
-static int recv_sb(struct client_task *ct, fd_set *rfds,
-		struct sb_buffer *result)
+static int recv_sb(struct client_task *ct, struct sb_buffer *result)
 {
 	int ret;
 	size_t n;
@@ -134,8 +133,6 @@ static int recv_sb(struct client_task *ct, fd_set *rfds,
 	void *trafo_context;
 	struct iovec iov;
 
-	if (!FD_ISSET(ct->scc.fd, rfds))
-		return 0;
 	if (ct->status < CL_SENT_CH_RESPONSE)
 		trafo = trafo_context = NULL;
 	else {
@@ -146,7 +143,7 @@ static int recv_sb(struct client_task *ct, fd_set *rfds,
 		ct->sbc[0] = sb_new_recv(0, trafo, trafo_context);
 again:
 	sb_get_recv_buffer(ct->sbc[0], &iov);
-	ret = read_nonblock(ct->scc.fd, iov.iov_base, iov.iov_len, rfds, &n);
+	ret = read_nonblock(ct->scc.fd, iov.iov_base, iov.iov_len, &n);
 	if (ret < 0) {
 		sb_free(ct->sbc[0]);
 		ct->sbc[0] = NULL;
@@ -288,7 +285,7 @@ static int client_post_select(struct sched *s, void *context)
 		return 0;
 	switch (ct->status) {
 	case CL_CONNECTED: /* receive welcome message */
-		ret = read_nonblock(ct->scc.fd, buf, sizeof(buf), &s->rfds, &n);
+		ret = read_nonblock(ct->scc.fd, buf, sizeof(buf), &n);
 		if (ret < 0 || n == 0)
 			goto out;
 		ct->features = parse_features(buf);
@@ -324,7 +321,7 @@ static int client_post_select(struct sched *s, void *context)
 		unsigned char crypt_buf[1024];
 		struct sb_buffer sbb;
 
-		ret = recv_sb(ct, &s->rfds, &sbb);
+		ret = recv_sb(ct, &sbb);
 		if (ret <= 0)
 			goto out;
 		if (sbb.band != SBD_CHALLENGE) {
@@ -371,7 +368,7 @@ static int client_post_select(struct sched *s, void *context)
 	case CL_SENT_CH_RESPONSE: /* read server response */
 		{
 		struct sb_buffer sbb;
-		ret = recv_sb(ct, &s->rfds, &sbb);
+		ret = recv_sb(ct, &sbb);
 		if (ret <= 0)
 			goto out;
 		free(sbb.iov.iov_base);
@@ -423,7 +420,7 @@ static int client_post_select(struct sched *s, void *context)
 				goto close0;
 			if (ret > 0 && FD_ISSET(ct->scc.fd, &s->rfds)) {
 				struct sb_buffer sbb;
-				ret = recv_sb(ct, &s->rfds, &sbb);
+				ret = recv_sb(ct, &sbb);
 				if (ret < 0)
 					goto close0;
 				if (ret > 0) {
diff --git a/dccp_recv.c b/dccp_recv.c
index 639c93fc..1773ffe7 100644
--- a/dccp_recv.c
+++ b/dccp_recv.c
@@ -118,7 +118,7 @@ static void dccp_recv_pre_select(struct sched *s, void *context)
 	para_fd_set(rn->fd, &s->rfds, &s->max_fileno);
 }
 
-static int dccp_recv_post_select(struct sched *s, void *context)
+static int dccp_recv_post_select(__a_unused struct sched *s, void *context)
 {
 	struct receiver_node *rn = context;
 	struct btr_node *btrn = rn->btrn;
@@ -136,7 +136,7 @@ static int dccp_recv_post_select(struct sched *s, void *context)
 	ret = -E_DCCP_OVERRUN;
 	if (iovcnt == 0)
 		goto out;
-	ret = readv_nonblock(rn->fd, iov, iovcnt, &s->rfds, &num_bytes);
+	ret = readv_nonblock(rn->fd, iov, iovcnt, &num_bytes);
 	if (num_bytes == 0)
 		goto out;
 	if (num_bytes <= iov[0].iov_len) /* only the first buffer was filled */
diff --git a/fd.c b/fd.c
index 63c0337b..f7e0e42b 100644
--- a/fd.c
+++ b/fd.c
@@ -177,14 +177,11 @@ __printf_2_3 int write_va_buffer(int fd, const char *fmt, ...)
  * \param fd The file descriptor to read from.
  * \param iov Scatter/gather array used in readv().
  * \param iovcnt Number of elements in \a iov.
- * \param rfds An optional fd set pointer.
  * \param num_bytes Result pointer. Contains the number of bytes read from \a fd.
  *
- * If rfds is not NULL and the (non-blocking) file descriptor fd is not set in
- * rfds, this function returns early without doing anything. Otherwise it tries
- * to read up to sz bytes from fd, where sz is the sum of the lengths of all
- * vectors in iov. Like \ref xwrite(), EAGAIN and EINTR are not considered
- * error conditions. However, EOF is.
+ * This function tries to read up to sz bytes from fd, where sz is the sum of
+ * the lengths of all vectors in iov. Like \ref xwrite(), EAGAIN and EINTR are
+ * not considered error conditions. However, EOF is.
  *
  * \return Zero or a negative error code. If the underlying call to readv(2)
  * returned zero (indicating an end of file condition) or failed for some
@@ -198,24 +195,12 @@ __printf_2_3 int write_va_buffer(int fd, const char *fmt, ...)
  *
  * \sa \ref xwrite(), read(2), readv(2).
  */
-int readv_nonblock(int fd, struct iovec *iov, int iovcnt, fd_set *rfds,
-		size_t *num_bytes)
+int readv_nonblock(int fd, struct iovec *iov, int iovcnt, size_t *num_bytes)
 {
 	int ret, i, j;
 
 	*num_bytes = 0;
-	/*
-	 * Avoid a shortcoming of select(): Reads from a non-blocking fd might
-	 * return EAGAIN even if FD_ISSET() returns true. However, FD_ISSET()
-	 * returning false definitely means that no data can currently be read.
-	 * This is the common case, so it is worth to avoid the overhead of the
-	 * read() system call in this case.
-	 */
-	if (rfds && !FD_ISSET(fd, rfds))
-		return 0;
-
 	for (i = 0, j = 0; i < iovcnt;) {
-
 		/* fix up the first iov */
 		assert(j < iov[i].iov_len);
 		iov[i].iov_base += j;
@@ -252,7 +237,6 @@ int readv_nonblock(int fd, struct iovec *iov, int iovcnt, fd_set *rfds,
  * \param fd The file descriptor to read from.
  * \param buf The buffer to read data to.
  * \param sz The size of \a buf.
- * \param rfds \see \ref readv_nonblock().
  * \param num_bytes \see \ref readv_nonblock().
  *
  * This is a simple wrapper for readv_nonblock() which uses an iovec with a single
@@ -260,10 +244,10 @@ int readv_nonblock(int fd, struct iovec *iov, int iovcnt, fd_set *rfds,
  *
  * \return The return value of the underlying call to readv_nonblock().
  */
-int read_nonblock(int fd, void *buf, size_t sz, fd_set *rfds, size_t *num_bytes)
+int read_nonblock(int fd, void *buf, size_t sz, size_t *num_bytes)
 {
 	struct iovec iov = {.iov_base = buf, .iov_len = sz};
-	return readv_nonblock(fd, &iov, 1, rfds, num_bytes);
+	return readv_nonblock(fd, &iov, 1, num_bytes);
 }
 
 /**
@@ -272,7 +256,6 @@ int read_nonblock(int fd, void *buf, size_t sz, fd_set *rfds, size_t *num_bytes)
  * \param fd The file descriptor to receive from.
  * \param pattern The expected pattern.
  * \param bufsize The size of the internal buffer.
- * \param rfds Passed to read_nonblock().
  *
  * This function tries to read at most \a bufsize bytes from the non-blocking
  * file descriptor \a fd. If at least \p strlen(\a pattern) bytes have been
@@ -284,11 +267,11 @@ int read_nonblock(int fd, void *buf, size_t sz, fd_set *rfds, size_t *num_bytes)
  *
  * \sa \ref read_nonblock(), \sa strncasecmp(3).
  */
-int read_pattern(int fd, const char *pattern, size_t bufsize, fd_set *rfds)
+int read_pattern(int fd, const char *pattern, size_t bufsize)
 {
 	size_t n, len;
 	char *buf = para_malloc(bufsize + 1);
-	int ret = read_nonblock(fd, buf, bufsize, rfds, &n);
+	int ret = read_nonblock(fd, buf, bufsize, &n);
 
 	buf[n] = '\0';
 	if (ret < 0)
diff --git a/fd.h b/fd.h
index 31b0c88c..37d3ace9 100644
--- a/fd.h
+++ b/fd.h
@@ -20,10 +20,9 @@ int para_munmap(void *start, size_t length);
 int read_ok(int fd);
 int write_ok(int fd);
 void valid_fd_012(void);
-int readv_nonblock(int fd, struct iovec *iov, int iovcnt, fd_set *rfds,
-		size_t *num_bytes);
-int read_nonblock(int fd, void *buf, size_t sz, fd_set *rfds, size_t *num_bytes);
-int read_pattern(int fd, const char *pattern, size_t bufsize, fd_set *rfds);
+int readv_nonblock(int fd, struct iovec *iov, int iovcnt, size_t *num_bytes);
+int read_nonblock(int fd, void *buf, size_t sz, size_t *num_bytes);
+int read_pattern(int fd, const char *pattern, size_t bufsize);
 int xwrite(int fd, const char *buf, size_t len);
 int xwritev(int fd, struct iovec *iov, int iovcnt);
 int for_each_file_in_dir(const char *dirname,
diff --git a/gui.c b/gui.c
index 7a859a0c..686fda31 100644
--- a/gui.c
+++ b/gui.c
@@ -621,7 +621,7 @@ static void status_pre_select(struct sched *s, void *context)
 		sched_request_barrier_or_min_delay(&st->next_exec, s);
 }
 
-static int status_post_select(struct sched *s, void *context)
+static int status_post_select(__a_unused struct sched *s, void *context)
 {
 	struct status_task *st = context;
 	size_t sz;
@@ -667,7 +667,7 @@ static int status_post_select(struct sched *s, void *context)
 	}
 	assert(st->loaded < st->bufsize);
 	ret = read_nonblock(st->fd, st->buf + st->loaded,
-		st->bufsize - st->loaded, &s->rfds, &sz);
+		st->bufsize - st->loaded, &sz);
 	st->loaded += sz;
 	ret2 = for_each_stat_item(st->buf, st->loaded, update_item);
 	if (ret < 0 || ret2 < 0) {
@@ -894,7 +894,7 @@ static void reread_conf(void)
 /* React to various signal-related events. */
 static int signal_post_select(struct sched *s, __a_unused void *context)
 {
-	int ret = para_next_signal(&s->rfds);
+	int ret = para_next_signal();
 
 	if (ret <= 0)
 		return 0;
@@ -942,7 +942,7 @@ static void exec_pre_select(struct sched *s, void *context)
 		sched_min_delay(s);
 }
 
-static int exec_post_select(struct sched *s, void *context)
+static int exec_post_select(__a_unused struct sched *s, void *context)
 {
 	struct exec_task *ct = context;
 	int i, ret;
@@ -963,7 +963,7 @@ static int exec_post_select(struct sched *s, void *context)
 			continue;
 		ret = read_nonblock(exec_fds[i],
 			ct->command_buf[i] + ct->cbo[i],
-			COMMAND_BUF_SIZE - 1 - ct->cbo[i], &s->rfds, &sz);
+			COMMAND_BUF_SIZE - 1 - ct->cbo[i], &sz);
 		ct->cbo[i] += sz;
 		sz = ct->cbo[i];
 		ct->cbo[i] = for_each_line(ct->flags[i], ct->command_buf[i],
diff --git a/http_recv.c b/http_recv.c
index 1fb60bad..42098b7e 100644
--- a/http_recv.c
+++ b/http_recv.c
@@ -105,7 +105,7 @@ static int http_recv_post_select(struct sched *s, void *context)
 		return 0;
 	}
 	if (phd->status == HTTP_SENT_GET_REQUEST) {
-		ret = read_pattern(rn->fd, HTTP_OK_MSG, strlen(HTTP_OK_MSG), &s->rfds);
+		ret = read_pattern(rn->fd, HTTP_OK_MSG, strlen(HTTP_OK_MSG));
 		if (ret < 0) {
 			PARA_ERROR_LOG("did not receive HTTP OK message\n");
 			goto out;
@@ -120,7 +120,7 @@ static int http_recv_post_select(struct sched *s, void *context)
 	iovcnt = btr_pool_get_buffers(rn->btrp, iov);
 	if (iovcnt == 0)
 		goto out;
-	ret = readv_nonblock(rn->fd, iov, iovcnt, &s->rfds, &num_bytes);
+	ret = readv_nonblock(rn->fd, iov, iovcnt, &num_bytes);
 	if (num_bytes == 0)
 		goto out;
 	if (num_bytes <= iov[0].iov_len) /* only the first buffer was filled */
diff --git a/http_send.c b/http_send.c
index c6b9decc..987145bf 100644
--- a/http_send.c
+++ b/http_send.c
@@ -170,7 +170,7 @@ static void http_post_select(fd_set *rfds, __a_unused fd_set *wfds)
 		case HTTP_STREAMING: /* nothing to do */
 			break;
 		case HTTP_CONNECTED: /* need to recv get request */
-			ret = read_pattern(sc->fd, HTTP_GET_MSG, MAXLINE, rfds);
+			ret = read_pattern(sc->fd, HTTP_GET_MSG, MAXLINE);
 			if (ret < 0)
 				phsd->status = HTTP_INVALID_GET_REQUEST;
 			else if (ret > 0) {
diff --git a/server.c b/server.c
index 729ca1ac..8c0dc44f 100644
--- a/server.c
+++ b/server.c
@@ -257,7 +257,7 @@ static int signal_post_select(struct sched *s, __a_unused void *context)
 	ret = task_get_notification(signal_task->task);
 	if (ret < 0)
 		return ret;
-	signum = para_next_signal(&s->rfds);
+	signum = para_next_signal();
 	switch (signum) {
 	case 0:
 		return 0;
diff --git a/signal.c b/signal.c
index 32d6ab66..27bda840 100644
--- a/signal.c
+++ b/signal.c
@@ -202,16 +202,14 @@ void para_unblock_signal(int sig)
 /**
  * Return the number of the next pending signal.
  *
- * \param rfds The fd_set containing the signal pipe.
- *
  * \return On success, the number of the received signal is returned. If there
  * is no signal currently pending, the function returns zero. On read errors
  * from the signal pipe, the process is terminated.
  */
-int para_next_signal(fd_set *rfds)
+int para_next_signal(void)
 {
 	size_t n;
-	int s, ret = read_nonblock(signal_pipe[0], &s, sizeof(s), rfds, &n);
+	int s, ret = read_nonblock(signal_pipe[0], &s, sizeof(s), &n);
 
 	if (ret < 0) {
 		PARA_EMERG_LOG("%s\n", para_strerror(-ret));
diff --git a/signal.h b/signal.h
index e5532ded..2f3422e2 100644
--- a/signal.h
+++ b/signal.h
@@ -38,7 +38,7 @@ struct signal_task *signal_init_or_die(void);
 void para_sigaction(int sig, void (*handler)(int));
 void para_install_sighandler(int);
 int para_reap_child(pid_t *pid);
-int para_next_signal(fd_set *rfds);
+int para_next_signal(void);
 void signal_shutdown(struct signal_task *st);
 void para_block_signal(int sig);
 void para_unblock_signal(int sig);
diff --git a/stdin.c b/stdin.c
index 9408235a..eea78b3e 100644
--- a/stdin.c
+++ b/stdin.c
@@ -37,7 +37,7 @@ static void stdin_pre_select(struct sched *s, void *context)
  * during the previous pre_select call. If so, and if STDIN_FILENO is readable,
  * data is read from stdin and fed into the buffer tree.
  */
-static int stdin_post_select(struct sched *s, void *context)
+static int stdin_post_select(__a_unused struct sched *s, void *context)
 {
 	struct stdin_task *sit = context;
 	ssize_t ret;
@@ -64,7 +64,7 @@ static int stdin_post_select(struct sched *s, void *context)
 	 * reference can not be freed, we're stuck.
 	 */
 	sz = PARA_MIN(sz, btr_pool_size(sit->btrp) / 2);
-	ret = read_nonblock(STDIN_FILENO, buf, sz, &s->rfds, &n);
+	ret = read_nonblock(STDIN_FILENO, buf, sz, &n);
 	if (n > 0)
 		btr_add_output_pool(sit->btrp, n, sit->btrn);
 	if (ret >= 0)
diff --git a/udp_recv.c b/udp_recv.c
index 58d45ab4..cc6e2049 100644
--- a/udp_recv.c
+++ b/udp_recv.c
@@ -68,7 +68,7 @@ static int udp_recv_post_select(__a_unused struct sched *s, void *context)
 	ret = -E_UDP_OVERRUN;
 	if (iovcnt == 0)
 		goto out;
-	ret = readv_nonblock(rn->fd, iov, iovcnt, &s->rfds, &num_bytes);
+	ret = readv_nonblock(rn->fd, iov, iovcnt, &num_bytes);
 	if (num_bytes == 0)
 		goto out;
 	readv_ret = ret;
-- 
2.39.2