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

[Rob Landley]

On 12/19/2017 03:38 PM, Rob Landley wrote:
> Sigh. Ok, it's been long enough I'm invoking the "apply the other
> person's patch then fiddle on top of it" rule, but this absolute path
> business is not what the other one's doing...

Darn it, in the attached patch I'm breaking out of the loop when a name
matches and calling callback() if the loop didn't exhaust the name list,
and I got to the "why does the callback have a return value", and
killall never does but pidof does and then I went:

  $ sleep 999 &
  [1] 3449
  $ pidof sleep sleep
  3449 3449

So nope, I can't break then handle. It's entirely possible I need a goto.

While I'm at it, I tried to write a one or two line comment for this and
just couldn't get it down to a reasonable size:

      if (!strcmp(bb, basename(cmd)) break;
      if (bb!=*cur && !strcmp(bb, basename(cmd+strlen(cmd)+1))) break;

The bb != *cur test is to make sure there's a path in argv[0], and I'm
not sure if I need it? Although #!blah without a path basically never
happens (but it works, runs an interpreter in the current directory) at
that point in the code we've already matched comm against basename (or
we would have continued at the top of the loop, we only call it a
_success_ if there's no path and strlen<16 but it's a _failure_ if it
doesn't match).

So the false positive case of not having that test would be something
like this passing:

  comm = thisisaverylong
  cmdline = thisisaverylongotherfile /usr/bin/thisisaverylongname

Which A) probably never happens, B) well what if that first one _was_ an
interpreter, C) how would only false positiving for
/opt/thisisaverylongotherfile be an improvement, D) exec can just make
_up_ the contents of argv[0] and argv[1] anyway so who am I kidding?

So I'm not sure what the heuristic accomplishes, but the _common_case_
is /bin/sh with an absolute path, so it _seems_ like something I should
check? (I suppose I could check for **cmd == '/' instead...)

(If you recall my discomfort with "just do what the other one does",
especially if I'm not reading any gnu/dammit code for
copyright/licensing reasons... What I care about is doing the right
thing. It's not always clear to me what that _is_. Consistency with
other implementation(s) is a strong weighting factor, but not the only one.)

> Rob

Seriously, my failure mode these days is writing something like this
patch and then throwing it away twice daily, because if I got distracted
from it now without doing the level of writeup and come back to look at
it in a week, all I'd remember is there were unresolved issues and then
I'd spent an hour staring at it trying to figure out what they were
without ever quite being comfortable with the result.

(And yes, the tiny things that don't matter are still the ones I get
hung up on, precisely because there's no clear winner.)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: grrr.patch
Type: text/x-patch
Size: 2444 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20171220/7177709f/attachment-0003.bin>