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

enh enh at google.com
Thu Jul 11 10:26:14 PDT 2019


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(-)



More information about the Toybox mailing list