On Thu, Dec 14, 2017 at 6:32 AM, Rob Landley <r...@landley.net> wrote: > 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.)
i don't like comm truncation either, but it still seems like the better test. it's more intention-revealing given that we don't have a canonical list of all valid interpreters. suppose you set $EDITOR or $PAGER to an absolute path. that shouldn't match. and, sure, comm is truncated, but argv[1] isn't, and comm is only used as a hint for "should i be looking at argv[1] rather than argv[0]?". plus checking comm seems to be consistent with other implementations, judging by strace. > (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. :) (use of `yes` copied from pgrep tests :-) ) > 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. :) `env python` is the other combination that springs to mind if you're trying for completeness. > Rob -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer. _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net