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?
Collin
