On Fri, 9 Dec 2022, Carl Edquist wrote:
A quick note, this check only needs to be done a total of once per
output, it shouldn't be done inside iopoll(), which would result in
an additional redundant fstat() per read().
Could this be handled by get_next_out?
Sure, either in that function or immediately after it gets called. But
also once for stdout before the while (n_outputs) loop. Alternatively,
allocate a file type array and populate it in the for loop that does all
the fopen() calls.
Patch attached for this. I ended up going with a bool array to keep track
of whether each output is a pipe or not. Potentially a very small amount
of extra work up front, but it seemed to make implementing the rest of it
simpler.
Carl
From 488e1398da6a15b70399d52346b05d9770db4732 Mon Sep 17 00:00:00 2001
From: Carl Edquist <edqu...@cs.wisc.edu>
Date: Sat, 10 Dec 2022 09:31:03 -0600
Subject: [PATCH] tee: only do iopoll for pipe outputs
We can skip the broken-pipe detection for file types other than pipes.
Sockets behave similarly to pipes in some ways, but the semantics
appear to be different in important ways, such that poll() doesn't
detect when the remote read end is closed.
* src/tee.c (is_pipe): add helper function detecting if fd is a pipe.
* (tee_files): populate out_is_pipe bool array corresponding to whether
items in descriptors array are pipes, only do pipe_check/iopoll() logic
for a given output if it is a pipe.
---
src/tee.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/tee.c b/src/tee.c
index b254466..482967a 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -276,6 +276,15 @@ fail_output(FILE **descriptors, char **files, int i)
return fail;
}
+/* Return true if fd refers to a pipe */
+
+static bool
+is_pipe(int fd)
+{
+ struct stat st;
+ return fstat (fd, &st) == 0 && S_ISFIFO(st.st_mode);
+}
+
/* Copy the standard input into each of the NFILES files in FILES
and into the standard output. As a side effect, modify FILES[-1].
Return true if successful. */
@@ -285,6 +294,7 @@ tee_files (int nfiles, char **files)
{
size_t n_outputs = 0;
FILE **descriptors;
+ bool *out_is_pipe;
char buffer[BUFSIZ];
ssize_t bytes_read = 0;
int i;
@@ -303,8 +313,10 @@ tee_files (int nfiles, char **files)
In both arrays, entry 0 corresponds to standard output. */
descriptors = xnmalloc (nfiles + 1, sizeof *descriptors);
+ out_is_pipe = xnmalloc (nfiles + 1, sizeof *out_is_pipe);
files--;
descriptors[0] = stdout;
+ out_is_pipe[0] = is_pipe (fileno (descriptors[0]));
files[0] = bad_cast (_("standard output"));
setvbuf (stdout, NULL, _IONBF, 0);
n_outputs++;
@@ -313,6 +325,7 @@ tee_files (int nfiles, char **files)
{
/* Do not treat "-" specially - as mandated by POSIX. */
descriptors[i] = fopen (files[i], mode_string);
+ out_is_pipe[i] = is_pipe (fileno (descriptors[i]));
if (descriptors[i] == NULL)
{
error (output_error == output_error_exit
@@ -329,7 +342,7 @@ tee_files (int nfiles, char **files)
while (n_outputs)
{
- if (pipe_check)
+ if (pipe_check && out_is_pipe[first_out])
{
int err = iopoll (STDIN_FILENO, fileno (descriptors[first_out]));
@@ -384,6 +397,7 @@ tee_files (int nfiles, char **files)
}
free (descriptors);
+ free (out_is_pipe);
return ok;
}
--
2.9.0
