On Wed, Apr 29, 2015 at 12:09 PM, Rob Landley <[email protected]> wrote: > On 04/25/2015 02:14 PM, enh wrote: >> what's the plan wrt SIGPIPE? > > Hadn't made any? I have xexit() doing fflush(0) || > ferror(stdout) and perror_msg("write") if you redirect > the output to a disk that fills up and such, but > getting killed by a signal has a default behavior > provided by the OS, and I left it alone... > >> the desktop is pretty inconsistent. >> many (but not all) commands install a signal handler that >> does _exit(0). others (coreutils 8.21's ls, say) do nothing. >> normally "what you do about SIGPIPE" isn't a problem but on >> Android that leads to a crash report and people filing "ls >> crashed" bugs against me. (our default shell PS setup is >> also noisy about crashes.) > > Is exiting due to sigpipe really a crash?
for toybox? probably not. for Angry Birds? definitely. for one of the command-line tools written in Java? perhaps. >> in the past, like the desktop, we've fixed this on an ad >> hoc basis. i just uploaded a change to toolbox >> (https://android-review.googlesource.com/148331) > > How do I get gerrit to show me the darn patch? I clicked > on the change ID and it opened the same "it touches these > two files and we'll show you how many lines were changed > but not _what_ changed" page... click on the filename in question. i don't think there's a way to see all files' changes at once... > If I right click on "gitiles" next to the commit ID it will > open a tree view of that version, but not a diff view. > > Ok, copy the change ID to the clipboard (horrible javascript > thing to turn it into an editing field as soon as I highlighted > it to prevent me from doing a normal cut and paste), then > right click open in new tab the project link in the upper right, > click on a random commit, paste the commit ID over that commit > number... nope, it's the same "what's a diff?" web interface... > > Ok, clone the darn git project and git show from the command > line. (Your web interface goes out of its way to be useless. > It puts _effort_ into being useless.) ...without using one or other of the various "download" options. > Ok, so it's basically: > > void handler(int i) {_exit(0);} > int main(int argc, char *argv[]) > { > signal(SIGPIPE, handler); > > Hmmm... ok, leaving aside your crash detector probably being > overzealous, another problem is toysh. It does some nofork() > commands and if they got a sigpipe the _shell_ would exit. > > So my first instinct (puting this in toy_singleinit()) is > inappropriate, and... I guess it really _does_ belong in > main(). Then toysh can set its own signal handlers and they > should stay set. (toy_init() isn't resetting signal > handlers. There's a gazillion subtle things about > toys.recursion that I'll need to audit someday, all of > which really translate to restrictions on who can call > toy_exec(). The man 2 execve page lists the things I need > to worry about.) > > Why are we calling _exit() instead of exit()? Does flush(0) > and calling the atexit() chain cause a problem here? (Modulo > it being done from the signal stack which might be a bit > cramped.) There's this whole nofork vague design about > being able to longjmp() from xexit() instead of exit... which > is of course where the above "gazillion subtle things" would > really come in, right now it's not a big deal because you > only really recurse deeply when you "nice nohup taskset setsid > chroot xargs time" and so on, and those generally aren't doing > anything fancy with state _other_ than what they're explicitly > there to manipulate... yeah, in toolbox there's nothing complicated to worry about, and _exit just gets me out of there cleanly and quickly. >> to do this in the >> driver instead. [pretend for a moment that toolbox isn't >> going away...] > > It's probably not going away by the Android M deadline, anyway. > >> any tool that actually cares about SIGPIPE behavior is >> unaffected because it can install its own SIGPIPE handler. >> any tool that doesn't care is unaffected because it's still >> going to exit; it's just going to exit cleanly. > > I'll defer to your definition of "cleanly" for android, > which means I'm adding it with an if (CFG_TOYBOX_ON_ANDROID) > as a bug workaround for what _sounds_ like an overzealous > crash detector. If I'm wrong about that and it should be > enabled generally, I can take the test out later once > people explain what I got wrong. (see the response i'm about to send to one of your later mails on this thread.) >> does this sound like a good idea for toybox? > > It sounds like something you need. > > Rob -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer. _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
