nudge on this attached patch
---------- Forwarded message ---------
From: Sam Liddicott <[email protected]>
Date: Fri, 13 Sep 2019 at 11:15
Subject: Re: SIGPIPE and tee
To: busybox <[email protected]>
Here is a patch which invokes kill_myself_with_sig(SIGPIPE) if fwrite fails
with errno=EPIPE.
Quitting with a signal rather than a diagnostic output emulates GNU tee
which is in line with the apparent reason for busybox tee in ignoring
SIGPIPE in the first place.
I have tested it with and without CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
As expected, dd succeeds for bs=1024 with CONFIG_FEATURE_TEE_USE_BLOCK_IO=y
but fails for without.
dd fails in both cases for bs=1024000
tee fails in both cases
( sleep 2 ; dd if=/dev/zero bs=1024000 count=5 ; echo dd $? > /dev/tty ; )
| {
build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
( sleep 2 ; dd if=/dev/zero bs=1024 count=5 ; echo dd $? > /dev/tty ; ) | {
build_output/sysroots-components/x86_64/busybox-native/bin/busybox.nosuid
tee /dev/null ; echo tee $? >/dev/tty ; } | sleep 1
A colleague has suggested that *any* write failure to *stdout* by tee ought
to cause tee to quit, but I leave this to your consideration; something
like this *might* cover both cases but I haven't tested it:
if (fwrite(buf, 1, c, *fp) != c) {
if (errno == EPIPE) kill_myself_with_sig(SIGPIPE);
retval = EXIT_FAILURE;
break;
}
Sam
On Thu, 12 Sep 2019 at 20:03, Sam Liddicott <[email protected]> wrote:
> In https://git.busybox.net/busybox/tree/coreutils/tee.c we read:
>
> /* gnu tee ignores SIGPIPE in case one of the output files is a pipe
> * that doesn't consume all its input. Good idea... */
> signal(SIGPIPE, SIG_IGN);
>
>
> Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting when
> stdout (as a pipe) is closed.
>
> Despite the comment, GNU tee does not behave as the comment suggests.
>
> Comparing 1.27.2-2ubuntu3.2 with tee (GNU coreutils) 8.28
>
> I test with one output file and stdout that stops early causing SIGPIPE
>
> 1. GNU tee
>
> ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( tee
> /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null
>
> 10+0 records in
> 10+0 records out
> 100 bytes copied, 0.000387276 s, 258 kB/s
> tee 141
> done 141
>
> the first two pipeline stages have exit code 141 due to sigpipe as we
> would expect
>
> 2. busybox tee
>
> ( dd if=/dev/zero bs=1024 count=8192 ; echo done $? >&2 ) | ( busybox tee
> /dev/null ; echo tee $? >&2) | dd bs=10 count=10 >/dev/null
> 10+0 records in
> 10+0 records out
> 100 bytes copied, 0.000291134 s, 343 kB/s
> 8192+0 records in
> 8192+0 records out
> 8388608 bytes (8.4 MB, 8.0 MiB) copied, 0.0447136 s, 188 MB/s
> done 0
> tee 0
>
> both prior pipeline elements have exit code zero and the first stage
> processed all of the data
>
> The POSIX spec for tee reads:
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/tee.html
> CONSEQUENCES OF ERRORS
>
> If a write to any successfully opened *file* operand fails, writes to
> other successfully opened *file* operands and standard output shall
> continue, but the exit status shall be non-zero. Otherwise, the default
> actions specified in *Utility Description Defaults*
> <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap01.html#tag_17_04>
> apply.
>
> But this case deals with a failure to write to stdout, not a failure to
> write to a *file* operand.
>
> Certainly, with this behaviour, busybox tee does not comply with GNU tee
> as it appears to intend, so I suggest that the line signal(SIGPIPE,
> SIG_IGN) be removed.
>
> To meet the probable intent, the signal should probably be ignored while
> writing to file operands (in case they turn out to be pipes) and enabled
> each time when writing to stdout.
>
> Sam
>
If we get EPIPE on write to stdout, then fail like a decent tee.
We fail with SIGPIPE like GNU tee as emulating GNU tee seems to be the aim.
--- ./coreutils/tee.c.o 2019-09-13 07:50:47.134804213 +0100
+++ ./coreutils/tee.c 2019-09-13 09:09:40.280881938 +0100
@@ -70,7 +70,9 @@
}
retval = EXIT_SUCCESS;
/* gnu tee ignores SIGPIPE in case one of the output files is a pipe
- * that doesn't consume all its input. Good idea... */
+ * that doesn't consume all its input. Good idea...
+ * An even better idea is to still respect SIGPIPE on stdout
+ * which we do further on. */
signal(SIGPIPE, SIG_IGN);
/* Allocate an array of FILE *'s, with one extra for a sentinel. */
@@ -100,9 +102,12 @@
#if ENABLE_FEATURE_TEE_USE_BLOCK_IO
while ((c = safe_read(STDIN_FILENO, buf, COMMON_BUFSIZE)) > 0) {
fp = files;
- do
+ if (fwrite(buf, 1, c, *fp) != c && errno == EPIPE) {
+ kill_myself_with_sig(SIGPIPE);
+ }
+ while (*++fp) {
fwrite(buf, 1, c, *fp);
- while (*++fp);
+ }
}
if (c < 0) { /* Make sure read errors are signaled. */
retval = EXIT_FAILURE;
@@ -111,9 +116,12 @@
setvbuf(stdout, NULL, _IONBF, 0);
while ((c = getchar()) != EOF) {
fp = files;
- do
+ if (putc(c, *fp) == EOF && errno == EPIPE) {
+ kill_myself_with_sig(SIGPIPE);
+ }
+ while (*++fp) {
putc(c, *fp);
- while (*++fp);
+ }
}
#endif
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
