[Toybox] [PATCH] killall should kill scripts too.

[Rob Landley]

On 12/13/2017 01:48 PM, enh wrote:
> Found running LTP file system tests on Android.

I have a todo item for this, but I was just going to also check argv[1]
when argv[0] is an absolute path (starts with '/'), and I should adjust
pgrep/pkill to do this too.

I'd rather not involve comm because it's capped at 15 chars (arbitrarily
truncated), I think the "treat absolute path as a potential interpreter"
logic is simpler and probably less brittle? (The "process could go away
between the two files we read" race probably isn't an issue here but not
having to care about that's a bonus.)

(Admittedly this is from somebody who's een doing 'ps ax | grep renderer
| awk '{print $1}' | xargs kill' for _years_ when chrome's eating too
much memory, and oddly enough getting away with it. I'm so glad it
started working again after random chrome update du jour fixed the
command line truncation bug a few months back :)

Hmmm, and we always need to do the basename() thing if we're going to
find ./thingy don't we? Hmmm, are we _sure_ libbuf is reliably null
terminated in the case of a pathological filename (could be
/big/long/path and sizeof(libbuf) == historical PATH_MAX), so... (It
hasn't hit us _yet_ because if we're the first user of libbuf it's gotta
be zeroed because it's in the BSS and the ELF spec says so, but as soon
as we're NOT the first user Bad Things Happen.)

And we need to getbasename() argv[1] too:

  $ ./sleep.sh &
  [1] 10840
  $ ps ax | grep sleep.sh
  10840 pts/38   S      0:00 /bin/bash ./sleep.sh
  10843 pts/38   S+     0:00 grep --color=auto sleep.sh

Except... when do you basename the command line argument?

  $ ps ax | grep vlc
  10881 ?        D      0:00 /usr/bin/vlc --started-from-file
  10883 pts/42   S+     0:00 grep --color=auto vlc
  $ killall /not/vlc
  /not/vlc: No such file or directory

Dear overcomplicated gnu/dammit utility: up yours. Not doing that,
whatever that is. Sigh, what crazy thing does posix say to do... it
doens't mention the existence of this utility. Well of course not.

Ok if I have a /path on the command line, don't basename. Otherwise
match basename. And NOW the fun of testing it! (Fire up qemu because
this is not the sort of thing I'm comfortable testing on my dev box...)

What new tests did you give me...

>  tests/killall.test | 14 +++++++++++++

Hmmm, I usually use "sleep" for these because it's self cleaning in the
failure case. :)

It'd be nice to have an executable test as well, but that's slightly
more awkward. (What do I expect to have install that's _not_ going to be
running on the host already? Ah, of course: symlink sleep under a new
name. :)

Rob