[Toybox] [PATCH] Auto-size df columns.

[enh]

Auto-size df columns.

On Android, the filesystem column is pretty wide. Actually measure the widths.

With this I'll switch AOSP over and we can see whether anyone
notices/cares that toybox calls realpath(3) on this column.

-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.
diff --git a/toys/posix/df.c b/toys/posix/df.c
index f5e2542..3b6e6d4 100644
--- a/toys/posix/df.c
+++ b/toys/posix/df.c
@@ -33,11 +33,55 @@ GLOBALS(
   struct arg_list *fstype;
 
   long units;
+  int column_widths[5];
+  int header_shown;
 )
 
-static void show_mt(struct mtab_list *mt)
+static void measure_column(int col, const char *s)
+{
+  size_t len = strlen(s);
+
+  if (TT.column_widths[col] < len) TT.column_widths[col] = len;
+}
+
+static void measure_numeric_column(int col, long long n)
+{
+  snprintf(toybuf, sizeof(toybuf), "%lld", n);
+  return measure_column(col, toybuf);
+}
+
+static void show_header()
+{
+  TT.header_shown = 1;
+
+  // The filesystem column is always at least this wide.
+  if (TT.column_widths[0] < 14) TT.column_widths[0] = 14;
+
+  if (toys.optflags & (FLAG_H|FLAG_h)) {
+xprintf("%-*s Size  Used Avail Use%% Mounted on\n",
+TT.column_widths[0], "Filesystem");
+  } else {
+const char *blocks_label = TT.units == 512 ? "512-blocks" : "1K-blocks";
+const char *use_label = toys.optflags & FLAG_P ? "Capacity" : "Use%";
+
+measure_column(1, blocks_label);
+measure_column(2, "Used");
+measure_column(3, "Available");
+measure_column(4, use_label);
+xprintf("%-*s %*s %*s %*s %*s Mounted on\n",
+TT.column_widths[0], "Filesystem",
+TT.column_widths[1], blocks_label,
+TT.column_widths[2], "Used",
+TT.column_widths[3], "Available",
+TT.column_widths[4], use_label);
+
+// For the "Use%" column, the trailing % should be inside the column.
+TT.column_widths[4]--;
+  }
+}
+
+static void show_mt(struct mtab_list *mt, int measuring)
 {
-  int len;
   long long size, used, avail, percent, block;
   char *device;
 
@@ -72,20 +116,32 @@ static void show_mt(struct mtab_list *mt)
   device = *mt->device == '/' ? realpath(mt->device, NULL) : NULL;
   if (!device) device = mt->device;
 
-  // Figure out appropriate spacing
-  len = 25 - strlen(device);
-  if (len < 1) len = 1;
-  if (toys.optflags & (FLAG_H|FLAG_h)) {
-char *size_str = toybuf, *used_str = toybuf+64, *avail_str = toybuf+128;
-int hr_flags = (toys.optflags & FLAG_H) ? HR_1000 : 0;
-
-human_readable(size_str, size, hr_flags);
-human_readable(used_str, used, hr_flags);
-human_readable(avail_str, avail, hr_flags);
-xprintf("%-16s%4s  %4s  %4s % 3lld%% %s\n", device,
-  size_str, used_str, avail_str, percent, mt->dir);
-  } else xprintf("%s% *lld % 10lld % 10lld % *lld%% %s\n", device, len,
-size, used, avail, (toys.optflags & FLAG_P) ? 7 : 3, percent, mt->dir);
+  if (measuring) {
+measure_column(0, device);
+measure_numeric_column(1, size);
+measure_numeric_column(2, used);
+measure_numeric_column(3, avail);
+  } else {
+if (!TT.header_shown) show_header();
+
+if (toys.optflags & (FLAG_H|FLAG_h)) {
+  char *size_str = toybuf, *used_str = toybuf+64, *avail_str = toybuf+128;
+  int hr_flags = (toys.optflags & FLAG_H) ? HR_1000 : 0;
+
+  human_readable(size_str, size, hr_flags);
+  human_readable(used_str, used, hr_flags);
+  human_readable(avail_str, avail, hr_flags);
+  xprintf("%-*s %4s  %4s  %4s % 3lld%% %s\n",
+TT.column_widths[0], device,
+size_str, used_str, avail_str, percent, mt->dir);
+} else xprintf("%-*s %*lld %*lld %*lld %*lld%% %s\n",
+TT.column_widths[0], device,
+TT.column_widths[1], size,
+TT.column_widths[2], used,
+TT.column_widths[3], avail,
+TT.column_widths[4], percent,
+mt->dir);
+  }
 
   if (device != mt->device) free(device);
 }
@@ -93,18 +149,13 @@ static void show_mt(struct mtab_list *mt)
 void df_main(void)
 {
   struct mtab_list *mt, *mtstart, *mtend;
-  int p = toys.optflags & FLAG_P;
+  int measuring;
 
-  // TODO: we don't actually know how wide the "Filesystem" column should be
-  // until we've looked at all the filesystems.
   if (toys.optflags & (FLAG_H|FLAG_h)) {
 TT.units = 1;
-xprintf("Filesystem  Size  Used Avail Use%% Mounted on\n");
   } else {
 // Units are 512 bytes if you select "pedantic" without "kilobytes".
-TT.units = p ? 512 : 1024;
-xprintf("Filesystem%8s-blocks\tUsed  Available %s Mounted on\n",
-  p ? "512" : "1K", p ? "Capacity" : "Use%");
+TT.units = toys.optflags & FLAG_P ? 512 : 1024;
   }
 
   if (!(mtstart = xgetmountlist(0))) return;
@@ -112,25 +163,28 @@ void df_main(void)
 
   // If we have a list of filesystems on the command line, loop through them.
   if (*toys.optargs) {
-char **next;
+// Measure the names then output the 

Re: [Toybox] ps -t is cheating.

[Rob Landley]

On 10/20/2015 12:43 AM, lamiaworks wrote:
> 
>> Message: 1
>> Date: Mon, 19 Oct 2015 00:09:17 -0500
>> From: Rob Landley 
>> To: toybox@lists.landley.net
>> Subject: [Toybox] ps -t is cheating.
>> Message-ID: <56247afd.5050...@landley.net>
>> Content-Type: text/plain; charset=utf-8
>>
>> So posix says this:
>>
>> -t  termlist
>>Write information for processes associated with terminals given in
>>termlist. The application shall ensure that the termlist is a single
>>argument in the form of a  or -separated list. Terminal
>>identifiers shall be given in an implementation-defined format. [XSI]
>>[Option Start]  On XSI-conformant systems, they shall be given in one
>>of two forms: the device's filename (for example, tty04) or, if the
>>device's filename starts with tty, just the identifier following the
>>characters tty (for example, "04" ). [Option End]
>>
>> And once again posix is somewhere back before the 1980's because
>> pseudo-terminals do not start with "/dev/tty". In the case of linux,
>> they've been /dev/pts/12 since, apparently, 1998.
>>
>> The ps man page says:
>>
>>-t ttylist
>> Select by tty.  This selects the processes associated with the
>> terminals given in ttylist.  Terminals (ttys, or screens for
>> text output) can be specified in several forms: /dev/ttyS1,
>> ttyS1, S1.  A plain "-" may be used to select processes not
>> attached to any terminal.
>>
>> Which is, once again, outright lying, because "-t 41" matches pts/41 but
>> -t 5 does _not_ match tty5. You have to say "-t tty5" to get the getty
>> instance on there.
> 
> You just brought an old nightmare back to the surface...
> 
> Try -t 05, if setup 'properly' you should get tty05 and that nowadays
> should be equating to tty5's inode / character settings.

  $ ps -t 05
  error: TTY could not be found

Linux hasn't got a /dev/tty05. It's got a /dev/tty5. But what procps's
version of ps does (and thus what people are going to expect) is show
pts/5 for digits.

I checked in code that treats purely numeric "-t 1" as a request for
/dev/pts/1 instead of /dev/tty1. To get tty1 say "-t tty1".

I dunno about "right" behavior. I just know that posix is decades behind
what linux is actually _doing_...

> My first Mainframes (1970's) had tty0 to ttyf for systems consoles I/O
> ONLY, and tty00 to ttyff for user terminals.

That would be the decades behind part. :)

> The problem was of course
> cost for the extra user i/o cards so any tty above tty3 were often
> assigned to users terminals and the relevant code 'fudged' to remove the
> console level access and assign user inodes, which in turn fudged other
> bits of codeet al...

Lovely.

I doubt android is providing any tty devices at all, with the possible
exception of an adb console, just pts devices to instances of the
"terminal" program. (Which isn't a default app on any of my phones yet,
but I live in hope that _someday_ I won't have to install a third party
app to get what seems to be built-in functionality.)

That said, the -t I checked in (yesterday?) is whitelisting specific
names, and if the adb terminal isn't among them I need to either stop
whitelisting (I worry that -t /dev/urandom or -t ../some/random/path
will somehow be exploitable, even though all I'm really doing is
stat-ing it) or add the new name...

>> Meanwhile procutils "ps -t pts/41" works as does "ps -t pts/../tty5"
>> which is just _creepy_ and I'm not doing that bit. And -tty S0 is of
>> course /dev/ttyS0 not /dev/pts/S0.
>>
>> I pine for a spec that means something,

Which means I really should _write_ the documentation I wanted to read,
yet again. Write a properly detailed "this is what toybox implements" as
an actual spec in the model of posix. Then wave it at the android and
tizen guys and have them frown and disagree with bits of it. And of
course fill in the tests; I haven't done proper ps tests yet because the
values change, from PIDs to %cpu output, but _how_ to do so is pretty
straightforward. Finite list of command line options and -o selections...

Alas, my open source stuff is not my day job. (I get to do some of it on
the clock, but the past couple week's been debugging nommu toolchains
and such...) Going as fast as I can in the time I have...

Rob
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] ps -t is cheating.

[enh]

On Wed, Oct 21, 2015 at 7:31 PM, Rob Landley  wrote:
>
>
> On 10/20/2015 12:43 AM, lamiaworks wrote:
>>
>>> Message: 1
>>> Date: Mon, 19 Oct 2015 00:09:17 -0500
>>> From: Rob Landley 
>>> To: toybox@lists.landley.net
>>> Subject: [Toybox] ps -t is cheating.
>>> Message-ID: <56247afd.5050...@landley.net>
>>> Content-Type: text/plain; charset=utf-8
>>>
>>> So posix says this:
>>>
>>> -t  termlist
>>>Write information for processes associated with terminals given in
>>>termlist. The application shall ensure that the termlist is a single
>>>argument in the form of a  or -separated list. Terminal
>>>identifiers shall be given in an implementation-defined format. [XSI]
>>>[Option Start]  On XSI-conformant systems, they shall be given in one
>>>of two forms: the device's filename (for example, tty04) or, if the
>>>device's filename starts with tty, just the identifier following the
>>>characters tty (for example, "04" ). [Option End]
>>>
>>> And once again posix is somewhere back before the 1980's because
>>> pseudo-terminals do not start with "/dev/tty". In the case of linux,
>>> they've been /dev/pts/12 since, apparently, 1998.
>>>
>>> The ps man page says:
>>>
>>>-t ttylist
>>> Select by tty.  This selects the processes associated with the
>>> terminals given in ttylist.  Terminals (ttys, or screens for
>>> text output) can be specified in several forms: /dev/ttyS1,
>>> ttyS1, S1.  A plain "-" may be used to select processes not
>>> attached to any terminal.
>>>
>>> Which is, once again, outright lying, because "-t 41" matches pts/41 but
>>> -t 5 does _not_ match tty5. You have to say "-t tty5" to get the getty
>>> instance on there.
>>
>> You just brought an old nightmare back to the surface...
>>
>> Try -t 05, if setup 'properly' you should get tty05 and that nowadays
>> should be equating to tty5's inode / character settings.
>
>   $ ps -t 05
>   error: TTY could not be found
>
> Linux hasn't got a /dev/tty05. It's got a /dev/tty5. But what procps's
> version of ps does (and thus what people are going to expect) is show
> pts/5 for digits.
>
> I checked in code that treats purely numeric "-t 1" as a request for
> /dev/pts/1 instead of /dev/tty1. To get tty1 say "-t tty1".
>
> I dunno about "right" behavior. I just know that posix is decades behind
> what linux is actually _doing_...
>
>> My first Mainframes (1970's) had tty0 to ttyf for systems consoles I/O
>> ONLY, and tty00 to ttyff for user terminals.
>
> That would be the decades behind part. :)
>
>> The problem was of course
>> cost for the extra user i/o cards so any tty above tty3 were often
>> assigned to users terminals and the relevant code 'fudged' to remove the
>> console level access and assign user inodes, which in turn fudged other
>> bits of codeet al...
>
> Lovely.
>
> I doubt android is providing any tty devices at all, with the possible
> exception of an adb console, just pts devices to instances of the
> "terminal" program. (Which isn't a default app on any of my phones yet,
> but I live in hope that _someday_ I won't have to install a third party
> app to get what seems to be built-in functionality.)

adb and Terminal.app both just use /dev/pts/*, yes. but Android
devices do tend to have a surprising number of /dev/tty* devices too.
here's AOSP Nexus 9, for example:

$ adb shell ls -l "/dev/tty*"
crw-rw-rw- 1 root  root   5,   0 2015-10-20 07:31 /dev/tty
crw--- 1 root  root 253,   0 2015-10-20 07:31 /dev/ttyFIQ0
crw--- 1 root  root 238,   0 2015-10-20 07:31 /dev/ttyGS0
crw--- 1 root  root 238,   1 2015-10-20 07:31 /dev/ttyGS1
crw--- 1 root  root 238,   2 2015-10-20 07:31 /dev/ttyGS2
crw--- 1 root  root 238,   3 2015-10-20 07:31 /dev/ttyGS3
crw--- 1 root  root 238,   4 2015-10-20 07:31 /dev/ttyGS4
crw--- 1 root  root   4,  64 2015-10-20 07:31 /dev/ttyS0
crw--- 1 root  root   4,  65 2015-10-20 07:31 /dev/ttyS1
crw--- 1 root  root   4,  66 2015-10-20 07:31 /dev/ttyS2
crw--- 1 root  root   4,  67 2015-10-20 07:31 /dev/ttyS3
crw--- 1 root  root 242,   0 2015-10-20 07:31 /dev/ttyTHS0
crw-rw 1 gps   system   242,   1 2015-10-20 07:31 /dev/ttyTHS1
crw-rw 1 bluetooth net_bt_stack 242,   2 2015-10-20 07:31 /dev/ttyTHS2
crw--- 1 root  root 242,   3 2015-10-20 07:31 /dev/ttyTHS3

> That said, the -t I checked in (yesterday?) is whitelisting specific
> names, and if the adb terminal isn't among them I need to either stop
> whitelisting (I worry that -t /dev/urandom or -t ../some/random/path
> will somehow be exploitable, even though all I'm really doing is
> stat-ing it) or add the new name...

your whitelist sounds good to me.

>>> Meanwhile procutils "ps -t pts/41" works as does "ps -t pts/../tty5"
>>> which is just