[prev in list] [next in list] [prev in thread] [next in thread]
List: busybox
Subject: Re: [PATCH] Re: SIGPIPE and tee
From: Sam Liddicott <sam () liddicott ! com>
Date: 2019-10-07 15:04:56
Message-ID: CAOj-5WAh8dBSykLg4mD1M6VKLx-ooQyz+J1-ciwWG5H-YXXGCQ () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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 <vda.linux@googlemail.com>
wrote:
> On Fri, Oct 4, 2019 at 11:20 AM Sam Liddicott <sam@liddicott.com> wrote:
> > nudge on this attached patch
> >
> > ---------- Forwarded message ---------
> > From: Sam Liddicott <sam@liddicott.com>
> > Date: Fri, 13 Sep 2019 at 11:15
> > Subject: Re: SIGPIPE and tee
> > To: busybox <busybox@busybox.net>
> >
> >
> > 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 <sam@liddicott.com> 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.
>
[Attachment #5 (text/html)]
<div dir="ltr">Thanks - that change is great. Busybox tee now propagates SIGPIPE just \
like GNU tee does by \
default.<br><div><div><div><br></div><div>Sam<br><div><br></div><div><br></div></div></div></div></div><br><div \
class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, 7 Oct 2019 at 13:27, \
Denys Vlasenko <<a \
href="mailto:vda.linux@googlemail.com">vda.linux@googlemail.com</a>> \
wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Fri, Oct 4, 2019 at \
11:20 AM Sam Liddicott <<a href="mailto:sam@liddicott.com" \
target="_blank">sam@liddicott.com</a>> wrote:<br> > nudge on this attached \
patch<br> ><br>
> ---------- Forwarded message ---------<br>
> From: Sam Liddicott <<a href="mailto:sam@liddicott.com" \
target="_blank">sam@liddicott.com</a>><br> > Date: Fri, 13 Sep 2019 at \
11:15<br> > Subject: Re: SIGPIPE and tee<br>
> To: busybox <<a href="mailto:busybox@busybox.net" \
target="_blank">busybox@busybox.net</a>><br> ><br>
><br>
> Here is a patch which invokes kill_myself_with_sig(SIGPIPE) if fwrite fails with \
errno=EPIPE.<br> ><br>
> 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.<br> <br>
<br>
><br>
> I have tested it with and without CONFIG_FEATURE_TEE_USE_BLOCK_IO=y<br>
><br>
> As expected, dd succeeds for bs=1024 with CONFIG_FEATURE_TEE_USE_BLOCK_IO=y but \
fails for without.<br> > dd fails in both cases for bs=1024000<br>
> tee fails in both cases<br>
><br>
> ( 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<br> ><br>
> ( 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<br> ><br>
> 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:<br> ><br>
> if (fwrite(buf, 1, c, *fp) != c) {<br>
> if (errno == EPIPE) kill_myself_with_sig(SIGPIPE);<br>
> retval = EXIT_FAILURE;<br>
> break;<br>
> }<br>
><br>
> Sam<br>
><br>
><br>
> On Thu, 12 Sep 2019 at 20:03, Sam Liddicott <<a \
href="mailto:sam@liddicott.com" target="_blank">sam@liddicott.com</a>> wrote:<br> \
>><br> >> In <a \
href="https://git.busybox.net/busybox/tree/coreutils/tee.c" rel="noreferrer" \
target="_blank">https://git.busybox.net/busybox/tree/coreutils/tee.c</a> we read:<br> \
>><br> >> /* gnu tee ignores SIGPIPE in case one of the output files is a \
pipe<br> >> * that doesn't consume all its input. Good idea... */<br>
>> signal(SIGPIPE, SIG_IGN);<br>
>><br>
>><br>
>> Sadly, this breaks POSIX SIGPIPE behaviour with respect to quitting when \
stdout (as a pipe) is closed.<br> >><br>
>> Despite the comment, GNU tee does not behave as the comment suggests.<br>
<br>
Exactly. GNU tee does not intercept SIGPIPE (unless -p is given).<br>
I'm removing SIGPIPE handling.<br>
Please try current git.<br>
</blockquote></div>
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic