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

>>> 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.
From 9c6fa92adb417b2e5f49ddd109b65cd985cd3558 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <[email protected]>
Date: Sat, 7 Nov 2015 09:44:10 -0800
Subject: [PATCH] Add ps -o CMDLINE.

This is equivalent to Android's historical "NAME" column, showing the
first element of /proc/pid/cmdline.
---
 toys/posix/ps.c | 82 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/toys/posix/ps.c b/toys/posix/ps.c
index 6af02a0..0f73d82 100644
--- a/toys/posix/ps.c
+++ b/toys/posix/ps.c
@@ -69,42 +69,43 @@ config PS
 
     Available -o FIELDs:
 
-      ADDR   Instruction pointer
-      CMD    Command line (including args)
-      COMM   Command name (no args)
-      ETIME  Elapsed time since process start
-      F      Process flags (PF_*) from linux source file include/sched.h
-             (in octal rather than hex because posix)
-      GID    Group id
-      GROUP  Group name
-      LABEL  Security label
-      MAJFL  Major page faults
-      MINFL  Minor page faults
-      NI     Niceness of process (lower niceness is higher priority)
-      PCPU   Percentage of CPU time used
-      PGID   Process Group ID
-      PID    Process ID
-      PPID   Parent Process ID
-      PRI    Priority
-      RGID   Real (before sgid) group ID
-      RGROUP Real (before sgid) group name
-      RSS    Resident Set Size (memory currently used)
-      RUID   Real (before suid) user ID
-      RUSER  Real (before suid) user name
-      S      Process state:
-             R (running) S (sleeping) D (disk sleep) T (stopped)  t (traced)
-             Z (zombie)  X (dead)     x (dead)       K (wakekill) W (waking)
-      STAT   Process state (S) plus:
-             < high priority          N low priority L locked memory
-             s session leader         + foreground   l multithreaded
-      STIME  Start time of process in hh:mm (size :19 shows yyyy-mm-dd hh:mm:ss)
-      SZ     Memory Size (4k pages needed to completely swap out process)
-      TIME   CPU time consumed
-      TTY    Controlling terminal
-      UID    User id
-      USER   User name
-      VSZ    Virtual memory size (1k units)
-      WCHAN  Waiting in kernel for
+      ADDR    Instruction pointer
+      CMD     Command line (from /proc/pid/cmdline, including args)
+      CMDLINE Command line (from /proc/pid/cmdline, no args)
+      COMM    Command name (from /proc/pid/stat, no args)
+      ETIME   Elapsed time since process start
+      F       Process flags (PF_*) from linux source file include/sched.h
+              (in octal rather than hex because posix)
+      GID     Group id
+      GROUP   Group name
+      LABEL   Security label
+      MAJFL   Major page faults
+      MINFL   Minor page faults
+      NI      Niceness of process (lower niceness is higher priority)
+      PCPU    Percentage of CPU time used
+      PGID    Process Group ID
+      PID     Process ID
+      PPID    Parent Process ID
+      PRI     Priority
+      RGID    Real (before sgid) group ID
+      RGROUP  Real (before sgid) group name
+      RSS     Resident Set Size (memory currently used)
+      RUID    Real (before suid) user ID
+      RUSER   Real (before suid) user name
+      S       Process state:
+              R (running) S (sleeping) D (disk sleep) T (stopped)  t (traced)
+              Z (zombie)  X (dead)     x (dead)       K (wakekill) W (waking)
+      STAT    Process state (S) plus:
+              < high priority          N low priority L locked memory
+              s session leader         + foreground   l multithreaded
+      STIME   Start time of process in hh:mm (size :19 shows yyyy-mm-dd hh:mm:ss)
+      SZ      Memory Size (4k pages needed to completely swap out process)
+      TIME    CPU time consumed
+      TTY     Controlling terminal
+      UID     User id
+      USER    User name
+      VSZ     Virtual memory size (1k units)
+      WCHAN   Waiting in kernel for
 */
 
 #define FOR_ps
@@ -354,7 +355,8 @@ static int do_ps(struct dirtree *new)
     // Command line limited to 2k displayable. We could dynamically malloc, but
     // it'd almost never get used, querying length of a proc file is awkward,
     // fixed buffer is nommu friendly... Wait for somebody to complain. :)
-    } else if (i==14) {
+    // CMDLINE - command line from /proc/pid/cmdline without arguments
+    } else if (i==14 || i==32) {
       int fd;
 
       len = 0;
@@ -365,7 +367,7 @@ static int do_ps(struct dirtree *new)
         if (0<(len = read(fd, out, 2047))) {
           if (!out[len-1]) len--;
           else out[len] = 0;
-          for (i = 0; i<len; i++) if (out[i] < ' ') out[i] = ' ';
+          if (i==14) for (i = 0; i<len; i++) if (out[i] < ' ') out[i] = ' ';
         }
         close(fd);
       }
@@ -425,13 +427,13 @@ static char *parse_o(char *type, int length)
          "F", "S", "UID", "PID", "PPID", "C", "PRI", "NI", "ADDR", "SZ",
          "WCHAN", "STIME", "TTY", "TIME", "CMD", "COMMAND", "ELAPSED", "GROUP",
          "%CPU", "PGID", "RGROUP", "RUSER", "USER", "VSZ", "RSS", "MAJFL",
-         "GID", "STAT", "RUID", "RGID", "MINFL", "LABEL"
+         "GID", "STAT", "RUID", "RGID", "MINFL", "LABEL", "CMDLINE"
   };
   // TODO: Android uses -30 for LABEL, but ideally it would auto-size.
   signed char widths[] = {1,-1,5,5,5,2,3,3,4+sizeof(long),5,
                           -6,5,-8,8,-27,-27,11,-8,
                           4,5,-8,-8,-8,6,5,6,
-                          8,-5,4,4,6,-30};
+                          8,-5,4,4,6,-30,-27};
   int i, j, k;
 
   // Get title, length of title, type, end of type, and display width
-- 
2.6.0.rc2.230.g3dd15c0

From e77506d727d91329f1e3541d9210e318c78de890 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <[email protected]>
Date: Sat, 7 Nov 2015 10:18:32 -0800
Subject: [PATCH] Add ps -n.

Both Android and GNU interpret -n to mean "show numeric users and groups",
despite what POSIX says.
---
 toys/posix/ps.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/toys/posix/ps.c b/toys/posix/ps.c
index 0f73d82..1e9b54c 100644
--- a/toys/posix/ps.c
+++ b/toys/posix/ps.c
@@ -6,7 +6,8 @@
  * And http://kernel.org/doc/Documentation/filesystems/proc.txt Table 1-4
  * And linux kernel source fs/proc/array.c function do_task_stat()
  *
- * Deviations from posix: no -n because /proc/self/wchan exists.
+ * Deviations from posix: no -n because /proc/self/wchan exists; we use -n to
+ * mean "show numeric users and groups" instead.
  * Posix says default output should have field named "TTY" but if you "-o tty"
  * the same field should be called "TT" which is _INSANE_ and I'm not doing it.
  * Similarly -f outputs USER but calls it UID (we call it USER).
@@ -26,7 +27,7 @@
  * TODO: ps aux (att & bsd style "ps -ax" vs "ps ax" behavior difference)
  * TODO: finalize F, remove C
  *       switch -fl to -y, use "string" instead of constants to set, remove C
- * TODO: --sort -Z
+ * TODO: --sort
  * TODO: way too many hardwired constants here, how can I generate them?
  * TODO: thread support /proc/$d/task/%d/stat (and -o stat has "l")
  *
@@ -34,13 +35,13 @@
  * significant. The array index is used in strawberry->which (consumed
  * in do_ps()) and in the bitmasks enabling default fields in ps_main().
 
-USE_PS(NEWTOY(ps, "P(ppid)*aAdeflo*p(pid)*s*t*u*U*g*G*wZ[!ol][+Ae]", TOYFLAG_USR|TOYFLAG_BIN))
+USE_PS(NEWTOY(ps, "P(ppid)*aAdeflno*p(pid)*s*t*u*U*g*G*wZ[!ol][+Ae]", TOYFLAG_USR|TOYFLAG_BIN))
 
 config PS
   bool "ps"
   default y
   help
-    usage: ps [-AadeflwZ] [-gG GROUP] [-o FIELD] [-p PID] [-t TTY] [-uU USER]
+    usage: ps [-AadeflnwZ] [-gG GROUP] [-o FIELD] [-p PID] [-t TTY] [-uU USER]
 
     List processes.
 
@@ -58,6 +59,10 @@ config PS
     -t	Attached to selected TTYs
     -u	Owned by USERs
     -U	Owned by real USERs (before suid)
+
+    Output modifiers:
+
+    -n	Show numeric USER and GROUP
     -w	Wide output (don't truncate at terminal width)
 
     Which FIELDs to show. (Default = -o PID,TTY,TIME,CMD)
@@ -256,7 +261,7 @@ static int do_ps(struct dirtree *new)
 
       // Even entries are numbers, odd are names
       sprintf(out, "%d", id);
-      if (i&1) {
+      if (!(toys.optflags&FLAG_n) && i&1) {
         if (i>3) {
           struct group *gr = getgrgid(id);
 
-- 
2.6.0.rc2.230.g3dd15c0

_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Reply via email to