On Sat, Nov 7, 2015 at 10:20 AM, enh <[email protected]> wrote: > On Thu, Nov 5, 2015 at 8:39 PM, Rob Landley <[email protected]> wrote: >> On 11/02/2015 12:51 PM, enh wrote: >>> On Mon, Nov 2, 2015 at 12:11 AM, Rob Landley <[email protected]> wrote: >>>> On 10/31/2015 04:02 PM, enh wrote: >>>>> Better for _me_ would be to change the current code that rewrites >>>>> characters < ' ' to leave '\0' alone if we're not supposed to be >>>>> outputting the arguments, but then toybox would match the Android >>>>> behavior (modulo the fact that we called this field NAME) rather than >>>>> the usual GNU behavior. >> >> >> >>>> Is this another documented deviation from posix in the big comment block >>>> near the top, or did I miss a curve? >>> >>> you mention one wart in this family: >>> >>> * It also says that -o "args" and "comm" should behave differently but use >>> * the same title, which is not the same title as the default output. (No.) >>> >>> but NAME is an Android invention. it's basically "imagine if -o comm >>> worked right and didn't truncate regardless of width". (and we've >>> never had -o, so it's the only one of the args/cmd/comm/command family >>> in use on Android.) >> >> Commands have two potential names though. >> >> There's the name the executable was originally called as, which is the >> parenthetical second argument from /proc/$PID/stat, and there's argv[0] >> from /proc/$PID/cmdline. If the process edits its argv[0], the >> /proc/$PID/cmdline stuff changes. (Note: changing the argv[0] pointer >> doesn't edit it, but reaching through that and changing the string >> contents would.) >> >> Also, a process's argv[0] often has a path on it. >> >> #include <stdio.h> >> >> int main(int argc, char *argv[]) >> { >> argv[0][2]='*'; >> sleep(1000); >> } >> gcc blah.c -o blah2 >> >> $ ps -o cmd,comm >> CMD COMMAND >> ./*lah2 blah2 >> ps -o cmd,comm ps >> bash bash >> >> I'm happy to take a patch adding NAME, but I can't write it myself >> without a little clarification about what you want it to do. :) > > attached is what i had in mind: > > ~/toybox$ ./toybox ps -o CMD,COMM,CMDLINE > CMD COMMAND CMDLINE > ./toybox ps -o CMD,COMM,CMD toybox ./toybox > -/bin/bash bash -/bin/bash > > i went with CMDLINE rather than our traditional NAME because it seemed > more intention-revealing (and we've never had -o, so it's not like > anyone's going to notice on our end). it keeps them together in the > help text too, which should make it easier for users to understand > their choices. > >>>>> Does anyone actually want the GNU truncating >>>>> behavior? cmd/comm/command seems such a mess that although I wouldn't >>>>> normally suggest adding new stuff, maybe just ignoring all this and >>>>> adding a NAME field isn't such a bad idea after all... >>>> >>>> I have no objection. The old posix default values are kinda nuts these >>>> days. >> >> I note that posix says "When the -o option is not specified, the >> standard output format is unspecified." and then goes on to define XSI >> extensions, which is way too much granularity for 2015. >> >> That said, I'm pretty happy to cange the default output and still call >> this ps posix complaint, because _they_ did. >> >>>> How any of that is usable "for scheduling", I couldn't tell you, since >>>> it's all historical usage of long-running processes telling you about >>>> what they did last week... >>> >>> Android has an unrelated non-standard set of scheduling-related >>> fields, but i was worrying about the portable stuff first. (i didn't >>> actually realize that -o comm was broken until working on this. i >>> thought our only deviation there was that we had a different name for >>> the field.) >> >> I'm pretty happy with the portable stuff. What else do you need now? >> >> My remaining big todo items for ps are: >> >> 1) bsd-style options >> 2) --sort >> 3) column autosizing ala ls >> >> (Note that 2 and 3 require the same preread infrastructure...) > > (-C might be nice, but i removed the equivalent [which didn't look for > -C took any non-integer argument as a process name] from Android a few > weeks back and no one's noticed yet.) > >>>> Anyway, I ripped out the crazy bitmap stuff and redid the default/-f/-l >>>> command line parsing as strings so it would be easy to change 'em. If >>>> you have a better idea what they should be, now's the time to speak up. >>>> (Heck, I've got a CFG_TOYBOX_ANDROID symbol, easy enough to change the >>>> default just for that, if you've got a userbase that'll care.) >>> >>> yeah, as far as i can tell from the internal tree basically 99% >>> existing use of Android's ps is just "ps" and then >>> grepping/sedding/parsing the result. >> >> Which is nice because android ps's other command line arguments are >> kinda nonstandard. >> >> The -nxc are new and don't conflict (partly because posix -n was stupid >> and I yanked it and documented the deviation), but -o is the standard >> way to add fields. (Same goes for -Z but we added that already.) >> >> The -P I added for ppid conflicts with -P policy (although I did ask >> first), and -p SHOW_PRIO conflicts with posix -p SHOW_PIDS (not my fault). > > i don't think we should waste flags on any of these (except -n). extra > -o labels is the right way to go. i'm 99% sure no one ever uses them > manually. they're used in the ps output that goes in every bug report > via dumpstate, but it's easy enough for me to supply a custom -o at > that one call site. > > interestingly, although the man page says otherwise, the desktop ps > supports -n and interprets it in the same way as Android. so i've > attached a patch for that too. > >> You've got thread support, that should go on the todo list above... >> >> exe_abi? *shrug* Ok. (It's not -o abi though, it's --abi.) And it _just_ >> says 32 or 64, no elf/binflt/fdpic, or arm/mips/x86? (I note that >> /proc/$PID/exe of shell scripts points to the interpreter executable, so >> that's fun. Of course the really fun code hiding is having your >> executable point to a custom dynamic linker and having that do all the >> work... :) > > yeah, i'm not really happy with what we have there at the moment. i > agree it should be a -o label, and did think that even though Android > doesn't have any x32 ABIs, others do, so if we add an equivalent of > --abi to toybox ps, we should try to be more general. this isn't a > priority though. it's only used interactively to answer questions like > "is <x> 64-bit?". (which is another reason why ABI is a terrible name; > on an x86 Android device you might have ARM code running via Houdini. > i don't know how we'd conveniently answer your "is Netflix ARM code or > x86?", but it's disappointing that --abi sounds like it might do so > but is actually unrelated.) > > ELFCLASS would have been more appropriate, though not as snappy. > >> Android's parsing of unrecognized arguments as pids is basically the >> same as what -p 1,2,3 does. Do you want that extension? (If so, should >> it grep for command names or something if it's not a pid?) You just >> atoi() and error out for zero... > > i think we can live without it, so we may as well insist that folks > use the POSIX syntax. > >> The android default output is: >> >> [LABEL] USER PID PPID VSIZE RSS [CPU] [PRIO] [POLICY] WCHAN PC [ABI] NAME >> >> Which with no arguments translates to: >> >> USER PID PPID VSIZE RSS WCHAN PC NAME >> >> Where NAME is argv[0] with the path and everything unless that's zero >> length, in which case you get the real name. > > can that ever be zero length? that's one thing that's different about > the toolbox implementation and the attached patch, but i can't think > how you'd ever hit that case. > >> USER shows username if >> we've got it, PC is eip. >> >> When you say grepping/sedding/parsing, are they chopping out field #3 or >> are they counting spaces? Because counting spaces means we depend on >> (and must preserve) the specific indentation... > > we've changed field widths enough that i doubt anyone's [still] > counting spaces, and if they are, we should help them escape that bad > habit. > >> A first pass at it seems to be to have android's default output be: >> ps -o user,pid,ppid,vsize,rss,wchan,addr:10=PC,comm=NAME > > our wchan is a little wider for readability, and we have the 's' field: > > toybox ps -o user,pid,ppid,vsize,rss,wchan:10,addr:10=PC,s,comm=NAME > > if you have the attached patch, this is closer still: > > toybox ps -o user,pid,ppid,vsize,rss,wchan:10,addr:10=PC,s,cmdline
for completeness, attached is the trivial patch for that. >>>> Also, on a previous patch: having --ppid like that in the help text is >>>> really ugly. Can we define -P as the short version of this so the help >>>> text doesn't mix longopts with shortopts? (Keeping the long alias for >>>> compatability with existing users, fine. Nothing seems to be using -P >>>> yet...) >>> >>> i did think it odd that GNU didn't have a short option for this >>> (unless they're just wary of stepping on future POSIX's toes), but >>> given that --ppid is the portable option i think that's the better >>> thing to have in the help text. >>> >>> (i'm not as anti-longopt as you in general. especially for rarely-used >>> functionality it's a lot easier to remember the longopt.) >> >> I think I talked about that last time. It's a preference, not a hard >> rule, but it makes help text parsing easier to standardize the format. >> But if we do have --longopts without short options they should go at the >> beginning or end to avoid confusing scripts/config2help.c. >> >>>> I'm trying to cut a release in the next day or two, so getting this >>>> nailed down would be nice... >>> >>> iirc, ps -C is the only standard thing that Android had an equivalent >>> of (with different syntax, of course). >>> >>> i think switching Android's ps over is going to be hard because it's >>> always been very different from regular ps *and* it's very heavily >>> used. i wouldn't block a release on that! >> >> Now that -o exists, users can in theory do a lot less parsing. Of course >> finding the users to convert them is the hard part. :) > > yeah, and for better or worse, "seeing what breaks" is often the only > real choice. > > it took three or four reverted attempts to get ls switched over, but > hopefully ps won't be any worse than that. > >> Rob > > -- > Elliott Hughes - http://who/enh - http://jessies.org/~enh/ > Android native code/tools questions? Mail me/drop by/add me as a reviewer. -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer.
From 6db600d0db44dcfc0cb8ec7af0451e951db377e8 Mon Sep 17 00:00:00 2001 From: Elliott Hughes <[email protected]> Date: Sat, 7 Nov 2015 10:32:13 -0800 Subject: [PATCH] On Android, ps' default output should match toolbox. --- toys/posix/ps.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/toys/posix/ps.c b/toys/posix/ps.c index 1e9b54c..f3ff39b 100644 --- a/toys/posix/ps.c +++ b/toys/posix/ps.c @@ -630,6 +630,8 @@ void ps_main(void) al.arg = "USER:8=UID,PID,PPID,C,STIME,TTY,TIME,CMD"; else if (toys.optflags&FLAG_l) al.arg = "F,S,UID,PID,PPID,C,PRI,NI,ADDR,SZ,WCHAN,TTY,TIME,CMD"; + else if (CFG_TOYBOX_ON_ANDROID) + al.arg = "USER,PID,PPID,VSIZE,RSS,WCHAN:10,ADDR:10=PC,S,CMDLINE"; else al.arg = "PID,TTY,TIME,CMD"; comma_args(&al, 0, parse_o); -- 2.6.0.rc2.230.g3dd15c0
_______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
