On 07/07/2015 06:00 PM, enh wrote: > rob: preferred fix here so we can provide you with the patch you want?
I'm still trying to unravel the top posting here... Let's see... > <[email protected]> wrote: > > Hi, > > I was trying toybox in nexus 9 android M preview and saw > that killall tries to kill "all" pid and pidof > whatever_you_type_without_a_slash_at_the_beginning answers > with a list containing all existing pids. > I am not sure if it belongs here or to the android bugtracker. killall ignoring its command line arguments would be a bug, yes. (I'm not sure what the pidof bit means, is this a separate bug in pidof or something about killall misparsing its output...?) $ ./pidof bash 1812 2316 2992 3327 3715 3787 4331 7102 7477 7520 7547 8637 8682 9209 9328 11539 11582 11616 11696 12066 16227 17118 17173 17230 18230 18329 18523 18634 18780 19151 22171 22311 22844 22960 23001 24135 24805 24895 26464 26560 27307 27606 28126 28156 28179 28313 28390 28911 30396 30418 30647 30688 30722 30791 31537 32328 32424 $ ./pidof init 1 1195 1864 It seems like toybox pidof is being selective without an absolute path...? > (this came up independently today when an OEM was surprised to find that > toybox's "killall" is rather too literal :-) ) Rather too broken, sounds like. (Pretty sure it worked when I tested it, although I might only have tested it against uclibc because I do that sort of thing in aboriginal under qemu, not on my host system...) > My findings: > > in lib.c line 819 > : !strcmp(basename(cmd), basename(*curname))) > > call to basename in both strcmp arguments does not work as > expected since the bionic basename gives back always the > same address for the returned char* > > I was not able to find a reliable source for the expected > behavior of baseline concerning the return buffer address > and allocation. Sigh. Posix warns that some implementations are... they're too polite to say "damn stupid" but: http://pubs.opengroup.org/onlinepubs/9699919799/functions/basename.html And of course we've been wrestling with this in commits 90e8605ea587 and 41ed97934989 and 7d64dae54bde and de699accf680 and e910826c812f and 468f155ecefe... (It SEEMS to do something so obvious! But unfortunately, due to the way posix defined it, it doesn't actually.) Grumble grumble basename_r() grumble. (Why would the gnu/dammit guys change the behavior of an existing function in a nonconforming way that varies based on which header you include first rather than doing the _r thing that 8 gazillion other non-threadsafe functions have done? How does that make sense?) Alright, I think I understand the problem now. And it being in a common lib.c function would explain pidof getting affected. And I think the fix is to just implement a basename_r() that returns "" when you have a trailing slash because _this_ is _crazy_. I checked in a wild guess at it, which seems to work for pidof. (Did pidof "" always list kernel threads? It does now. The other pidof lists nothing, but doesn't consider it an error, so...) Let me know if I missed anything important in the rest of the inside-out top posted message? Thanks, Rob _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
