[Toybox] [PATCH] Fix the ps -o CMD vs COMM distinction.

[Rob Landley]

On 11/02/2015 12:51 PM, enh wrote:
> On Mon, Nov 2, 2015 at 12:11 AM, Rob Landley <rob at landley.net> 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. :)

>>> 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...)

>> 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).

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... :)

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...

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. 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...

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

>> 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. :)

Rob

 1446784776.0