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

[enh]

On Tue, Dec 19, 2017 at 1:38 PM, Rob Landley <rob at landley.net> wrote:
>
>
> On 12/18/2017 07:07 PM, enh wrote:
>> On Sun, Dec 17, 2017 at 9:40 AM, Rob Landley <rob at landley.net> wrote:
>>> On 12/17/2017 11:20 AM, Rob Landley wrote:
>>>> Sigh. You argument that "we can't just do something easily explained but
>>>> must copy ubuntu's magic edge cases exactly" implies I need to
>>>> understand ubuntu's magic edge cases, which are nonobvious.
>>>
>>> I should clarify: what I'm uncomfortable with is the need to read
>>> multiple /proc files per process, because the speed of lsof is just
>>> _sad_ and the cpu usage of top is unpleasant. (Part of the reason I
>>> haven't finished cleaning up lsof yet is I still hope to be able to
>>> speed it up somehow, which is a largeish time sink every time I turn my
>>> attention to it.)
>>>
>>> Having killall need to do similar grinding over a large number of
>>> processes seems unnecessary. That said, it looks like to match ubuntu's
>>> killall we would need to read two files _and_ stat /proc/$$/exe.
>>
>> that's still a lot lighter weight than all the work pgrep/pkill have
>> to do, and it's what everyone's already living with anyway...
>
> That and the "it doesn't have to be right or match a spec, it has to
> match the other command's behavior", which is a nonobvious hairball once
> I started looking at it:
>
>   $ echo -e '#!/bin/bash\nsleep "$@"' > spleep
>   $ chmod +x spleep
>   $ spleep 123 &
>   [1] 28391
>   $ killall $PWD/spleep
>   /home/landley/toybox/toy3/sub2/spleep: no process found
>
> Because no process has /proc/*/exe pointing to that file's dev:inode
> pair. The existing lib/ plumbing for this is _only_ checking names, not
> checking this inode stuff. But this is in lib because it's shared
> plumbing, pidof also uses it and the host version of that has -s to
> check for scripts...
>
> 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...

thanks. though you have though managed to convince me that the
situation is already a mess.

i think i might only have tested against busybox killall, not GNU
killall (because the original bug report said "busybox works" and
didn't mention GNU), and i just assumed busybox and GNU matched.

> Rob



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.