[Toybox] [PATCH] stat - support %m, %t, %T, format for files and added -t, -L options

[Rob Landley]

On 05/12/2016 08:20 AM, Igor Kalkov wrote:
> Hi,
> 
> I was going to submit a similar patch next week. I'm glad Sameer did
> this work already, thanks!

Or possibly Bilal did this work already, according to the first line the
patch would add?

> Any progress on this patch?

I just got back from 2 weeks in Japan, and went off caffeine on my
travels back so basically spent the next 2 days sleeping. (My big plans
to get work done on the flight home did not survive contact with
United's economy class.)

Looking at the patch, this adds -t and -L but only adds -t to the usage:
line (and doesn't collate [-ft] but instead is [-t] [-f] and then
gratuitously rewraps all the help text to make the column sizes larger
(seems unnecessary)...

I don't understand this bit:

@@ -55,31 +61,29 @@
 {
   strftime(toybuf, sizeof(toybuf), "%Y-%m-%d %H:%M:%S",
     localtime(&(ts->tv_sec)));
-  xprintf("%s.%09ld", toybuf, ts->tv_nsec);
+  xprintf("%s.%09d", toybuf, ts->tv_nsec);
 }

I just rebuilt with both a glibc x86-64 toolchain and a musl 32 bit
toolchain and neither complained about %ld here? I think this change
would _add_ a warning...? (I vaguely recall an android issue about this,
but alas, my google-fu is failing me. Specifically, I thought the
various android 32/64 bit size issues were _fixed_, in N if not in M?)

Sigh: back in c89 all arguments were promoted to "int" so printf and
friends didn't have this trouble, then when 64 bits came along they went
"oh we couldn't possibly waste that much space, back thenthey only had
kilobytes of memory and thought type promotion was a good idea, these
days we have gigabytes of memory and must _conserve_ them" and thus the
great int/long/longlong type micromanagement race started.

Ever wanted to smack a standards committee?

Let's see...

  git clone https://android.googlesource.com/platform/bionic
  cd bionic
  git annotate libc/include/bits/timespec.h | grep tv_nsec

db499037  ( Yabin Cui  2014-12-09 20:15:48 -0800  42)  long tv_nsec;

So it's been "long" for at least a couple years now?

Next up there's the gratuitous removal of the function out() which...
Ah, I see! This patch reverts the stat.c portions of commit d3a435e53c94
from January.

Yeah, not applying that as-is. Lemme try to come up with something.

> It would be really helpful to have the -t and -L options.

Oh agreed, just not _this_ patch. :)

I'll try to come up with something. (That patch also only honors -L for
stat and not for statfs, but I think that's ok...? %m re-stats when we
have "stat" in print_stat already...)

Does stat REALLY use the capital letter for minor and the lower case
letter for major? Yes, yes it does. Who wrote this?

Hmmm... There's no tests/stat.test file. And looking at the output of
"./stat stat" I can see why:

  $ ./stat stat
    File: `stat'
    Size: 18936	 Blocks: 40	 IO Blocks: 4096	regular file
  Device: 801h/2049d	 Inode: 11928631	 Links: 1
  Access: (555/-r-xr-xr-x)	Uid: (1000/ landley)	Gid: (1000/ landley)
  Access: 2016-05-15 08:54:01.236289275
  Modify: 2016-05-15 08:51:54.608292570
  Change: 2016-05-15 08:51:54.612292570

There's not a whole lot generic and portable in there. Hmmm. I should do
a separate post about testing strategy.

Um, what _is_ the definition of %m? My first instinct was to do
xabspath() and then compare directories, but this patch is testing
st_dev (and st_rdev) which is funky if you stop and think about how you
can mount the same filesystem in multiple places (since the 2.5 kernel).

This will show the most recent mount point, but "mount /dev/sdb1 /mnt"
followed by "/dev/sdb1 /media" will show /media for files under /mnt. In
fact, the way this is written I _think_ if you bind mount /dev to
/home/me/blah and then "stat -c %m /dev/tty2" it'll show /home/me/blah
because a shadow of the _containing_ mount hits first. (The dev rather
than the rdev.)

Huh, now I just tried ubuntu's stat and no matter what /dev node I feed
it, it shows /dev as the mount point. So it doesn't look like ubuntu's
stat isn't using rdev at all...?

Sigh, what busybox have I got lying around... 1.21.1. And that says that
"stat -c %m /dev/sda1" is "m". Great.

I'm going with the xabspath() check, I think. Since what it's supposed
to show is the mount point above this file, and that should get the
right answer for that question.

Except... how does -L work into this? Presumably you'd _not_ resolve
symlinks with -L? Which implies that if /bin is a symlink to /usr/bin
and /usr is a mountpoint, "stat /bin" without -L would say / instead of
/usr? Which would be correct in that instance because the /bin symlink
lives on / but then /bin/ls would also say / when that would be /usr
because ls is a file living in /usr...

Also, when you _do_ resolve a symlink, there's no guarantee it points to
anything that actually exists.

Grrr. What is the correct behavior here? There's no spec!

> Regards,
> Igor Kalkov

Confused,

Rob