Re: [PATCH] ls: support --time=creation to show/sort birth time

[Assaf Gordon]

Hello,

On 2020-01-02 2:01 p.m., Pádraig Brady wrote:
> On 02/01/2020 20:29, Assaf Gordon wrote:
> > Regarding "fall back to mtime", I'm seeing the following results
> > on some systems - not necessarily a bug, but perhaps it's worth
> > knowing what to expect:
> >
> > * Debian 10/x86_64, Linux Kernel 4.19.0, glibc 2.28-10,
> > with ext2 file system (not supporting birthtime):
> >
> >     $ ./src/ls -l --time=birth /tmp/dummy-ext2/2
> >     -rw-r--r-- 1 root root 0 Dec 31  1969 /tmp/dummy-ext2/2
> Hmm. That suggests that STATX_BTIME is set in the
> returned statx mask, but populated with 0 in the structure
> ({-1,-1} would have printed as '?'). Though you say src/stat
> prints '-' in all cases, and the logic should be much the same.
> Could you confirm the birth time significant returns for this case.
> epoch isn't a bad time to output in this case, but it would
> be good to be consistent.

Indeed, the returned "btime" is zero:

----
$ strace -v -e trace=statx ./src/stat /tmp/dummy-ext2/2
statx(AT_FDCWD, "/tmp/dummy-ext2/2", 
AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW, STATX_ALL, 
{stx_mask=STATX_BASIC_STATS, stx_blksize=1024, stx_attributes=0, 
stx_nlink=1, stx_uid=0, stx_gid=0, stx_mode=S_IFREG|0644, stx_ino=12, 
stx_size=0, stx_blocks=0, 
stx_attributes_mask=STATX_ATTR_COMPRESSED|STATX_ATTR_IMMUTABLE|STATX_ATTR_APPEND|STATX_ATTR_NODUMP|STATX_ATTR_ENCRYPTED, 
stx_atime={tv_sec=1577995860, tv_nsec=0} /* 2020-01-02T13:11:00-0700 */, 
stx_btime={tv_sec=0, tv_nsec=0}, stx_ctime={tv_sec=1577995860, 
tv_nsec=0} /* 2020-01-02T13:11:00-0700 */, stx_mtime={tv_sec=1577995860, 
tv_nsec=0} /* 2020-01-02T13:11:00-0700 */, stx_rdev_major=0, 
stx_rdev_minor=0, stx_dev_major=7, stx_dev_minor=0}) = 0
  File: /tmp/dummy-ext2/2
  Size: 0               Blocks: 0          IO Block: 1024   regular empty file
Device: 700h/1792d      Inode: 12          Links: 1
Access: (0644/-rw-r--r--)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2020-01-02 13:11:00.000000000 -0700
Modify: 2020-01-02 13:11:00.000000000 -0700
Change: 2020-01-02 13:11:00.000000000 -0700
 Birth: -
+++ exited with 0 +++

$ strace -v -e trace=statx ./src/ls -l --time=birth /tmp/dummy-ext2/2
statx(AT_FDCWD, "/tmp/dummy-ext2/2", 
AT_STATX_SYNC_AS_STAT|AT_SYMLINK_NOFOLLOW, 
STATX_MODE|STATX_NLINK|STATX_UID|STATX_GID|STATX_SIZE|STATX_BTIME, 
{stx_mask=STATX_BASIC_STATS, stx_blksize=1024, stx_attributes=0, 
stx_nlink=1, stx_uid=0, stx_gid=0, stx_mode=S_IFREG|0644, stx_ino=12, 
stx_size=0, stx_blocks=0, 
stx_attributes_mask=STATX_ATTR_COMPRESSED|STATX_ATTR_IMMUTABLE|STATX_ATTR_APPEND|STATX_ATTR_NODUMP|STATX_ATTR_ENCRYPTED, 
stx_atime={tv_sec=1577995860, tv_nsec=0} /* 2020-01-02T13:11:00-0700 */, 
stx_btime={tv_sec=0, tv_nsec=0}, stx_ctime={tv_sec=1577995860, 
tv_nsec=0} /* 2020-01-02T13:11:00-0700 */, stx_mtime={tv_sec=1577995860, 
tv_nsec=0} /* 2020-01-02T13:11:00-0700 */, stx_rdev_major=0, 
stx_rdev_minor=0, stx_dev_major=7, stx_dev_minor=0}) = 0
-rw-r--r-- 1 root root 0 Dec 31  1969 /tmp/dummy-ext2/2
+++ exited with 0 +++
----

Looking closer at the new ls.c code (** are added for emphasis):
---
do_statx (int fd, const char *name, struct stat *st, int flags, 

          unsigned int mask) 

{ 

  struct statx stx; 

**bool want_btime = mask & STATX_BTIME; 

  int ret = statx (fd, name, flags, mask, &stx); 

  if (ret >= 0) 

    { 

      statx_to_stat (&stx, st); 

      /* Since we only need one timestamp type, 

         store birth time in st_mtim.  */ 

**    if (mask & STATX_BTIME) 

        st->st_mtim = statx_timestamp_to_timespec (stx.stx_btime); 

**   else if (want_btime) 

        st->st_mtim.tv_sec = st->st_mtim.tv_nsec = -1; 

    } 



  return ret; 

---

Wouldn't "mask & STATX_BTIME" always be the same as "want_btime",
resulting in the "else if" part never to be executed?

IIUC, "mask" is the requested bitmask.

Comparing with "stat.c:do_stat()", I see:

    ...
    statx_to_stat (&stx, &st); 

    if (stx.stx_mask & STATX_BTIME) 

       pa.btime = statx_timestamp_to_timespec (stx.stx_btime); 

    ...

Which I recon is the returned bitmask (vs requested bitmask).

Could that be the issue ?


-assaf