Simplify managing the pollfd array.

src/vdfp_pipe.c

@@ -103,12 +103,16 @@ static struct vdp_tree tree_h;
 struct vdp_state {
 	unsigned		magic;
 #define PIPE_VDP_STATE_MAGIC 0xaeb87f5f
+	struct pollfd		fds[3];
 	struct VPFX(pipe_vdp)	*obj;
 	char			*buf;
 	pid_t			chldpid;
-	int			chldin;
-	int			chldout;
-	int			chlderr;
+};
+
+static const char *stream_name[] = {
+	[STDIN_FILENO]  = "stdin",
+	[STDOUT_FILENO] = "stdout",
+	[STDERR_FILENO] = "stderr",
 };
 
 /* VDP */
@@ -179,9 +183,9 @@ vdp_init(struct req *req, void **priv)
 	*priv = state;
 	state->obj = obj;
 	state->chldpid = -1;
-	state->chldin = -1;
-	state->chldout = -1;
-	state->chlderr = -1;
+	state->fds[STDIN_FILENO].events = POLLOUT;
+	state->fds[STDOUT_FILENO].events = POLLIN;
+	state->fds[STDERR_FILENO].events = POLLOUT;
 
 	if (mk_pipe(in, obj->name, req->vsl) != 0)
 		return (-1);
@@ -230,35 +234,19 @@ vdp_init(struct req *req, void **priv)
 	closefd(&in[0]);
 	closefd(&out[1]);
 	closefd(&err[1]);
-	state->chldin = in[1];
-	state->chldout = out[0];
-	state->chlderr = err[0];
+	state->fds[STDIN_FILENO].fd = in[1];
+	state->fds[STDOUT_FILENO].fd = out[0];
+	state->fds[STDERR_FILENO].fd = err[0];
 	http_Unset(req->resp, H_Content_Length);
 	return (0);
 }
 
 static inline void
-close_all(struct vdp_state *state)
-{
-	if (state->chldin != -1)
-		closefd(&state->chldin);
-	if (state->chldout != -1)
-		closefd(&state->chldout);
-	if (state->chlderr != -1)
-		closefd(&state->chlderr);
-}
-
-static inline const char *
-stream_name(int fd, struct vdp_state *state)
+close_all(struct pollfd *fds)
 {
-	if (fd == state->chldin)
-		return "stdin";
-	if (fd == state->chldout)
-		return "stdout";
-	if (fd == state->chlderr)
-		return "stderr";
-	WRONG("fd matches no child stream");
-	return (NULL);
+	for (int i = 0; i < 3; i++)
+		if (fds[i].fd != -1)
+			closefd(&fds[i].fd);
 }
 
 /* Assumes CHECK_OBJ* for state called by caller. */
@@ -282,7 +270,7 @@ check_pid(struct vdp_state *state, struct vsl_log *vsl, int options)
 	}
 	assert(pid == state->chldpid);
 	state->chldpid = -1;
-	close_all(state);
+	close_all(state->fds);
 	if (WIFEXITED(status)) {
 		if (WEXITSTATUS(status) == 0) {
 			VSLb(vsl, SLT_Notice, "vdfp_pipe: vdp %s: %s exited "
@@ -315,43 +303,25 @@ static int
 rw_child(struct req *req, struct vdp_state *state, enum vdp_action act,
 	 const void *ptr, ssize_t len)
 {
-	struct pollfd fds[3];
-	int retval, nfds;
+	struct pollfd *fds;
+	int retval;
 	ssize_t nbytes;
 
 	CHECK_OBJ_NOTNULL(state->obj, PIPE_VDP_MAGIC);
 	AN(state->obj->name);
 	AN(state->obj->path);
+	fds = state->fds;
 
 	if (act == VDP_END && len == 0)
-		closefd(&state->chldin);
+		closefd(&fds[STDIN_FILENO].fd);
 
 	for (;;) {
-		nfds = 0;
-		if (len > 0 && state->chldin != -1) {
-			AN(ptr);
-			fds[0].fd = state->chldin;
-			fds[0].events = POLLOUT;
-			fds[0].revents = 0;
-			nfds++;
-		}
-		if (state->chldout != -1) {
-			fds[nfds].fd = state->chldout;
-			fds[nfds].events = POLLIN;
-			fds[nfds].revents = 0;
-			nfds++;
-		}
-		if (state->chlderr != -1) {
-			fds[nfds].fd = state->chlderr;
-			fds[nfds].events = POLLIN;
-			fds[nfds].revents = 0;
-			nfds++;
-		}
-		if (nfds == 0)
-			break;
+		fds[STDIN_FILENO].revents = 0;
+		fds[STDOUT_FILENO].revents = 0;
+		fds[STDERR_FILENO].revents = 0;
 
 		errno = 0;
-		retval = poll(fds, nfds, TIMEOUT_MS);
+		retval = poll(fds, 3, TIMEOUT_MS);
 		if (retval < 0) {
 			assert(errno == EINTR);
 			continue;
@@ -364,37 +334,36 @@ rw_child(struct req *req, struct vdp_state *state, enum vdp_action act,
 		}
 		else if (retval == 0)
 			continue;
-		for (int i = 0; i < retval; i++) {
+
+		for (int i = 0; i < 3; i++) {
 			if (fds[i].revents == 0)
 				continue;
 			AZ(fds[i].revents & POLLNVAL);
 			if (fds[i].revents & POLLERR) {
 				VSLb(req->vsl, SLT_Error, "vdfp_pipe: vdp %s: "
 				     "error polling %s %s", state->obj->name,
-				     state->obj->path,
-				     stream_name(fds[i].fd, state));
-				close_all(state);
+				     state->obj->path, stream_name[i]);
+				close_all(fds);
 				return (-1);
 			}
-			if (fds[i].revents & POLLHUP
-			    && fds[i].fd == state->chldin) {
-				closefd(&state->chldin);
+			if (fds[i].revents & POLLHUP && i == STDIN_FILENO) {
+				closefd(&fds[STDIN_FILENO].fd);
 				continue;
 			}
 			if (fds[i].revents & POLLOUT) {
-				assert(fds[i].fd == state->chldin);
-				AN(ptr);
-				assert(len > 0);
+				assert(i == STDIN_FILENO);
+				if (len == 0 || ptr == NULL)
+					continue;
 
 				errno = 0;
-				nbytes = write(state->chldin, ptr, len);
+				nbytes = write(fds[STDIN_FILENO].fd, ptr, len);
 				if (nbytes < 0) {
 					VSLb(req->vsl, SLT_Error,
 					     "vdfp_pipe: vdp %s: error writing "
 					     "to %s stdin: %s",
 					     state->obj->name, state->obj->path,
 					     vstrerror(errno));
-					close_all(state);
+					close_all(fds);
 					return (-1);
 				}
 				AN(nbytes);
@@ -403,14 +372,13 @@ rw_child(struct req *req, struct vdp_state *state, enum vdp_action act,
 				ptr += nbytes;
 				continue;
 			}
-			assert(fds[i].fd == state->chldout
-			       || fds[i].fd == state->chlderr);
+			assert(i == STDOUT_FILENO || i == STDERR_FILENO);
 			if (!(fds[i].revents & POLLIN)) {
 				AN(fds[i].revents & POLLHUP);
-				if (fds[i].fd == state->chldout)
-					closefd(&state->chldout);
+				if (i == STDOUT_FILENO)
+					closefd(&fds[STDOUT_FILENO].fd);
 				else
-					closefd(&state->chlderr);
+					closefd(&fds[STDERR_FILENO].fd);
 				continue;
 			}
 			errno = 0;
@@ -418,25 +386,23 @@ rw_child(struct req *req, struct vdp_state *state, enum vdp_action act,
 			if (nbytes < 0) {
 				VSLb(req->vsl, SLT_Error, "vdfp_pipe: vdp %s:"
 				     " error reading %s from %s: %s",
-				     state->obj->name,
-				     stream_name(fds[i].fd, state),
-				     state->obj->path,
-				     vstrerror(errno));
-				close_all(state);
+				     state->obj->name, stream_name[i],
+				     state->obj->path, vstrerror(errno));
+				close_all(fds);
 				return (-1);
 			}
 			if (nbytes == 0) {
-				if (fds[i].fd == state->chldout)
-					closefd(&state->chldout);
+				if (i == STDOUT_FILENO)
+					closefd(&fds[STDOUT_FILENO].fd);
 				else
-					closefd(&state->chlderr);
+					closefd(&fds[STDERR_FILENO].fd);
 				continue;
 			}
-			if (fds[i].fd == state->chldout) {
+			if (i == STDOUT_FILENO) {
 				retval = VDP_bytes(req, VDP_FLUSH, state->buf,
 						   nbytes);
 				if (retval < 0) {
-					close_all(state);
+					close_all(fds);
 					return (-1);
 				}
 				continue;
@@ -447,9 +413,10 @@ rw_child(struct req *req, struct vdp_state *state, enum vdp_action act,
 			VSLb_bin(req->vsl, SLT_Error, nbytes, state->buf);
 		}
 		if (act == VDP_END) {
-			if (state->chldin != -1)
-				closefd(&state->chldin);
-			if (state->chldout != -1 || state->chlderr != -1)
+			if (fds[STDIN_FILENO].fd != -1)
+				closefd(&fds[STDIN_FILENO].fd);
+			if (fds[STDOUT_FILENO].fd != -1 ||
+			    fds[STDERR_FILENO].fd != -1)
 				continue;
 			break;
 		}
@@ -488,7 +455,7 @@ vdp_fini(struct req *req, void **priv)
 	AN(state->obj->name);
 	AN(state->obj->path);
 
-	close_all(state);
+	close_all(state->fds);
 
 	if (state->chldpid != -1)
 		(void)check_pid(state, req->vsl, 0);