[Toybox] [PATCH] killall: better handling of long names.

[enh]

okay, so three hours of churning through build servers and test farms,
that did pass... i've sent "[PATCH] pidof: fix default behavior, add
-x." which should be production-quality. in particular, it addresses
what i was calling "#3" above.

On Thu, Jul 11, 2019 at 10:26 AM enh <enh at google.com> wrote:
>
> so while i still think this patch is an improvement for killall... it
> changed the behavior of pidof (the other user of names_to_pid) in a
> bad way. (which breaks some test automation.)
>
> basically, `pidof blah` (where blah is a running process) now returns
> a match for the pidof call too, because names_to_pid now actually
> looks at argv[1] where it didn't before. killall works around this by
> having an extra `pid != getpid()` check. so solution #1 is that i
> could add that to pidof...
>
> ...but procps pidof doesn't look at argv[1] in any circumstances
> afaict. so solution #2 is that we add a flag to names_to_pid that says
> whether or not it should look at argv[1]; killall passes true, pidof
> passes false...
>
> ...but i think killall should be more selective anyway:
>
> /tmp$ cat no-op.c
> #include <unistd.h>
> int main(int argc, char* argv[]) {
>   return sleep(666);
> }
> /tmp$ make no-op && ./no-op this-is-unlikely &
> cc     no-op.c   -o no-op
> [1]+ ./no-op this-is-unlikely-and-long &
> /tmp$ pidof this-is-unlikely-and-long
> /tmp$ killall -i this-is-unlikely-and-long
> this-is-unlikely-and-long: no process found
> /tmp$
>
> versus toybox:
>
> /tmp$ ~/toybox-pidof/toybox pidof this-is-unlikely-and-long
> 249522 249619
> /tmp$ ~/toybox-pidof/toybox killall -i this-is-unlikely-and-long
> Signal this-is-unlikely-and-long(249522) (y/N):n
> killall: this-is-unlikely-and-long: No such process
> /tmp$
>
> so whatever the procps killall heuristic for "is this a shell script?"
> is, it's more fussy than just "check argv[1]". (having a stricter
> heuristic that doesn't get non-shell-scripts would be solution #3.)
>
> i'm testing #1 now, just to make sure this is my _only_ current
> problem preventing me from updating toybox. except `pidof pidof`
> *does* return something, so "real" pidof doesn't do this.
>
> okay, scratch #1 then. let's try #2...
>
> On Mon, Jul 1, 2019 at 4:21 PM enh <enh at google.com> wrote:
> >
> > Change names_to_pid() so that we can actually match shell scripts with
> > long names (the code to get the shell script's name was correct, but
> > there was an extra test preventing us from actually comparing it to the
> > sought name).
> >
> > In kill.c itself, remove a dead test for -l and switch to the FLAG()
> > macro.
> >
> > Also extend the tests to explicitly cover long and short names.
> > ---
> >  lib/lib.c          |  2 +-
> >  tests/killall.test | 12 +++++++++---
> >  toys/lsb/killall.c | 12 ++++++------
> >  3 files changed, 16 insertions(+), 10 deletions(-)