subject:"Re\: SIGPIPE and tee"

Re: [PATCH] Re: SIGPIPE and tee

2019-10-07 Thread Sam Liddicott

Thanks - that change is great. Busybox tee now propagates SIGPIPE just like
GNU tee does by default.

Sam



On Mon, 7 Oct 2019 at 13:27, Denys Vlasenko 
wrote:

> On Fri, Oct 4, 2019 at 11:20 AM Sam Liddicott  wrote:
> > nudge on this attached patch
> >
> > -- Forwarded message -
> > From: Sam Liddicott 
> > Date: Fri, 13 Sep 2019 at 11:15
> > Subject: Re: SIGPIPE and tee
> > To: busybox 
> >
> >
> > 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  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.
>
> Exactly. GNU tee does not intercept SIGPIPE (unless -p is given).
> I'm removing SIGPIPE handling.
> Please try current git.
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Re: SIGPIPE and tee

2019-10-07 Thread Denys Vlasenko

On Fri, Oct 4, 2019 at 11:20 AM Sam Liddicott  wrote:
> nudge on this attached patch
>
> -- Forwarded message -
> From: Sam Liddicott 
> Date: Fri, 13 Sep 2019 at 11:15
> Subject: Re: SIGPIPE and tee
> To: busybox 
>
>
> 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  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.

Exactly. GNU tee does not intercept SIGPIPE (unless -p is given).
I'm removing SIGPIPE handling.
Please try current git.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

[PATCH] Re: SIGPIPE and tee

2019-10-04 Thread Sam Liddicott

nudge on this attached patch

-- Forwarded message -
From: Sam Liddicott 
Date: Fri, 13 Sep 2019 at 11:15
Subject: Re: SIGPIPE and tee
To: busybox 


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  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_s

Re: SIGPIPE and tee

2019-09-13 Thread Sam Liddicott

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  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*
> 
>  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
>


0001-Add-SIGPIPE-handler-to-tee.patch
Description: Binary data
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: SIGPIPE and tee

2019-09-13 Thread Sam Liddicott

I also note that busybox tee treats a filename single hyphen as stdout,
contrary to POSIX.

I mention it only because it might save a few bytes to remove this special
case, and treat it as a regular file.

https://pubs.opengroup.org/onlinepubs/9699919799/utilities/tee.html
OPERANDS

The following operands shall be supported:
*file*A pathname of an output file. If a *file* operand is '-', it shall
refer to a file named *-*; implementations shall not treat it as meaning
standard output.


On Thu, 12 Sep 2019 at 20:03, Sam Liddicott  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*
> 
>  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
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Re: [PATCH] Re: SIGPIPE and tee

Re: [PATCH] Re: SIGPIPE and tee

[PATCH] Re: SIGPIPE and tee

Re: SIGPIPE and tee

Re: SIGPIPE and tee

5 matches

Site Navigation

Mail list logo

Footer information