works for me, thanks. the comment isn't quite right though. patch attached.
On Fri, Feb 10, 2017 at 2:39 PM, Rob Landley <[email protected]> wrote: > On 02/09/2017 03:50 PM, enh wrote: >> this just came up again. the (internal) bug report being "`ps -A | >> head -n1` shouldn't die with SIGPIPE". >> >> i think from this thread we agreed that there shouldn't be any >> top-level SIGPIPE handler, with grep being an example where SIGPIPE is >> the least worst choice. > > /me reads quoted message... > > Ah right, bionic adds a handler to turn this into an error message, then > doesn't want it to be an error for toybox. Ok then. > > It looks like this commit: > > commit 483cfdabaf6ab282987a0a21d6177c3daa95dc12 > Author: Rob Landley <[email protected]> > Date: Sun May 3 20:18:53 2015 -0500 > > Replace android-specific hack with just signal(SIGPIPE, SIG_IGN). > > Got accidentally reverted in: > > commit 3b51a07e478d64a84e40b3a7c026b2f8566b194b > Author: Rob Landley <[email protected]> > Date: Sun Sep 27 09:03:41 2015 -0500 > > Another chunk of nommu support, replacing toys.recursion with > toys.stacktop. > > Which doesn't even mention SIGPIPE. > > That said, SIGDFL is probably better than SIGIGN because we _do_ want it > to kill the process. If we let toybox do the ferror() check on the way > out it could report its own error condition. > >> so what about the specific case of ps? (as reported previously, >> desktop ps seems to install a signal handler and _exit(0).) > > "Specific case of ps" is a red flag for me, seems like a generic problem > needing a generic solution. The fflush() shenanigans in xprintf() and > xexit() are so this: > > $ ./ls -l > /dev/full > ls: write: No space left on device > > Gets caught. Not having ps/ls/cat report an error with SIGPIPE seems > similarly generic. > > Rob -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer.
From 94f9b3db01019f0f10a4db51f209b2bc85b6cfa9 Mon Sep 17 00:00:00 2001 From: Elliott Hughes <[email protected]> Date: Wed, 15 Feb 2017 16:42:42 -0800 Subject: [PATCH] Fix the comment about the Android SIGPIPE behavior. --- main.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/main.c b/main.c index a2267da..bb1b646 100644 --- a/main.c +++ b/main.c @@ -213,8 +213,11 @@ int main(int argc, char *argv[]) } *argv = getbasename(*argv); - // Bionic's dynamic linker adds a handler to report SIGPIPE as an error, - // then doesn't want that behavior for toybox. So disable it for bionic. + // Up to and including Android 6.0 (Marshmallow), bionic's dynamic linker + // added a handler to cause a crash dump on SIGPIPE. That was removed in + // Android 7.0 (Nougat), but Android still gets bug reports about the + // regular "ps: write: broken pipe" output when someone pipes the output + // of ps into head. if (CFG_TOYBOX_ON_ANDROID) signal(SIGPIPE, SIG_DFL); // If nommu can't fork, special reentry path. -- 2.11.0.483.g087da7b7c-goog
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
