On 15/03/2026 20:52, Collin Funk wrote:
Pádraig Brady <[email protected]> writes:
On 15/03/2026 03:45, Collin Funk wrote:
I wasn't too sure why 'tee' used streams to be honest. It looks like
it was for an option that was never implemented:
commit 8ddf2904779fefb47e4caa68632341a22f18cffa
Author: Jim Meyering <[email protected]>
AuthorDate: Mon Jul 26 07:11:27 1999 +0000
Commit: Jim Meyering <[email protected]>
CommitDate: Mon Jul 26 07:11:27 1999 +0000
(tee): Convert from open/fds to using fopen/streams
for
output, in preparation for addition of new compression option.
My main rationale for this patch though is to make it easier to
experiment with things like splice.
I think this is a good change, as I previously considered:
https://lists.gnu.org/archive/html/coreutils/2015-03/msg00012.html
Oh, I didn't know about that. Like you mention it is a micro
optimization. The only way I could see a noticeable change in
performance was with an invocation like this:
$ dd if=/dev/random of=input bs=1024 count=1000000 status=none
$ time tee $(yes /dev/null | head -n 1000) < input > /dev/null
real 0m33.399s
user 0m13.922s
sys 0m19.316s
$ time ./src/tee $(yes /dev/null | head -n 1000) < input > /dev/null
real 0m28.001s
user 0m8.734s
sys 0m19.146s
But with typical invocations, the performance seemed to be unchanged.
I.e., the only difference seemed to be the time it takes to read/write
varying between invocations.
extern bool
-fwrite_wait (char const *buf, ssize_t size, FILE *f)
+write_wait (int fd, void const *buffer, size_t size)
{
- for (;;)
+ unsigned char const *buf = buffer;
+
+ while (true)
{
- const size_t written = fwrite (buf, 1, size, f);
+ ssize_t written = write (fd, buf, size);
+ if (written < 0)
+ {
+ if (errno == EINTR)
+ continue;
+ written = 0;
+ }
+
We could use safe_write() here,
but why the new consideration for EINTR at all?
Otherwise LGTM.
I hadn't really thought much about the EINTR case, to be honest. I put
it there because tee_files() has:
bytes_read = read (STDIN_FILENO, buffer, sizeof buffer);
if (bytes_read < 0 && errno == EINTR)
continue;
The 'git blame' says that those lines are from 33 years ago, so I
probably shouldn't have placed much weight on it as things may have been
different then.
I guess they should both be safe to remove, since we don't use any
signal handlers. Is that correct?
Yes, that's what I was thinking.
I.e. remove the EINTR handling on the write case anyway
so that the only functionality change in the patch
is the stdio removal.
Removing EINTR on the read path would be another change,
which I'm not sure about TBH.
cheers,
Padraig
