diff mbox series

[3/5] linux: Use statx for MIPSn64

Message ID 20210319183121.2252064-4-adhemerval.zanella@linaro.org
State New
Headers show
Series More stat fixes | expand

Commit Message

Adhemerval Zanella March 19, 2021, 6:31 p.m. UTC 
MIPSn64 kernel ABI for legacy stat uses unsigned 32 bit for second
timestamp, which limits the maximum value to y2106.  This patch
make mips64 use statx as for 32-bit architectures.

Thie __cp_stat64_t64_statx is open coded, its usage is solely on
fstatat64 and it avoid the need to redefine the name for mips64
(which will call __cp_stat64_statx since its does not use
__stat64_t64 internally).
---
 sysdeps/unix/sysv/linux/fstatat64.c        | 29 +++++++++++++++++++---
 sysdeps/unix/sysv/linux/fxstat64.c         |  1 +
 sysdeps/unix/sysv/linux/mips/kernel_stat.h |  4 +++
 sysdeps/unix/sysv/linux/statx_cp.c         | 29 ----------------------
 4 files changed, 30 insertions(+), 33 deletions(-)

Comments

Adhemerval Zanella March 29, 2021, 12:48 p.m. UTC | #1 
Since other patches have been reviewed, I will commit this shortly.
I ran the tst-futimens, tst-utime, and tst-utimes on a mips64-le
with both a 4.1.4 and 5.10 kernel and got the expected results.

On 19/03/2021 15:31, Adhemerval Zanella wrote:
> MIPSn64 kernel ABI for legacy stat uses unsigned 32 bit for second
> timestamp, which limits the maximum value to y2106.  This patch
> make mips64 use statx as for 32-bit architectures.
> 
> Thie __cp_stat64_t64_statx is open coded, its usage is solely on
> fstatat64 and it avoid the need to redefine the name for mips64
> (which will call __cp_stat64_statx since its does not use
> __stat64_t64 internally).
> ---
>  sysdeps/unix/sysv/linux/fstatat64.c        | 29 +++++++++++++++++++---
>  sysdeps/unix/sysv/linux/fxstat64.c         |  1 +
>  sysdeps/unix/sysv/linux/mips/kernel_stat.h |  4 +++
>  sysdeps/unix/sysv/linux/statx_cp.c         | 29 ----------------------
>  4 files changed, 30 insertions(+), 33 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
> index c3a030af58..b9b8cd994b 100644
> --- a/sysdeps/unix/sysv/linux/fstatat64.c
> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
> @@ -24,9 +24,9 @@
>  #include <kernel_stat.h>
>  #include <sysdep.h>
>  #include <time.h>
> -#include <statx_cp.h>
>  #include <kstat_cp.h>
>  #include <stat_t64_cp.h>
> +#include <sys/sysmacros.h>
>  
>  #if __TIMESIZE == 64 \
>       && (__WORDSIZE == 32 \
> @@ -50,8 +50,28 @@ fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf,
>    struct statx tmp;
>    int r = INTERNAL_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT | flag,
>  				 STATX_BASIC_STATS, &tmp);
> -  if (r == 0)
> -    __cp_stat64_t64_statx (buf, &tmp);
> +  if (r != 0)
> +    return r;
> +
> +  *buf = (struct __stat64_t64) {
> +    .st_dev = makedev (tmp.stx_dev_major, tmp.stx_dev_minor),
> +    .st_rdev = makedev (tmp.stx_rdev_major, tmp.stx_rdev_minor),
> +    .st_ino = tmp.stx_ino,
> +    .st_mode = tmp.stx_mode,
> +    .st_nlink = tmp.stx_nlink,
> +    .st_uid = tmp.stx_uid,
> +    .st_gid = tmp.stx_gid,
> +    .st_atime = tmp.stx_atime.tv_sec,
> +    .st_atim.tv_nsec = tmp.stx_atime.tv_nsec,
> +    .st_mtime = tmp.stx_mtime.tv_sec,
> +    .st_mtim.tv_nsec = tmp.stx_mtime.tv_nsec,
> +    .st_ctime = tmp.stx_ctime.tv_sec,
> +    .st_ctim.tv_nsec = tmp.stx_ctime.tv_nsec,
> +    .st_size = tmp.stx_size,
> +    .st_blocks = tmp.stx_blocks,
> +    .st_blksize = tmp.stx_blksize,
> +  };
> +
>    return r;
>  }
>  
> @@ -117,7 +137,8 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>  }
>  
>  #if (__WORDSIZE == 32 \
> -     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
> +     || defined STAT_HAS_TIME32
>  # define FSTATAT_USE_STATX 1
>  #else
>  # define FSTATAT_USE_STATX 0
> diff --git a/sysdeps/unix/sysv/linux/fxstat64.c b/sysdeps/unix/sysv/linux/fxstat64.c
> index be12798273..23d9d92b00 100644
> --- a/sysdeps/unix/sysv/linux/fxstat64.c
> +++ b/sysdeps/unix/sysv/linux/fxstat64.c
> @@ -25,6 +25,7 @@
>  #include <xstatconv.h>
>  #include <statx_cp.h>
>  #include <shlib-compat.h>
> +#include <sys/sysmacros.h>
>  
>  #if LIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_33)
>  

This is not required.

> diff --git a/sysdeps/unix/sysv/linux/mips/kernel_stat.h b/sysdeps/unix/sysv/linux/mips/kernel_stat.h
> index e4b0f211ca..19524f7ea4 100644
> --- a/sysdeps/unix/sysv/linux/mips/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/mips/kernel_stat.h
> @@ -67,5 +67,9 @@ struct kernel_stat
>  #else
>  # define STATFS_IS_STATFS64 0
>  #endif
> +/* MIPS64 has unsigned 32 bit timestamps fields, so use statx as well.  */
> +#if _MIPS_SIM == _ABI64
> +# define STAT_HAS_TIME32
> +#endif
>  
>  #endif
> diff --git a/sysdeps/unix/sysv/linux/statx_cp.c b/sysdeps/unix/sysv/linux/statx_cp.c
> index 53068704c6..73405a9612 100644
> --- a/sysdeps/unix/sysv/linux/statx_cp.c
> +++ b/sysdeps/unix/sysv/linux/statx_cp.c
> @@ -48,32 +48,3 @@ __cp_stat64_statx (struct stat64 *to, struct statx *from)
>  }
>  #endif
>  
> -#if (__WORDSIZE == 32 \
> -     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
> -void
> -__cp_stat64_t64_statx (struct __stat64_t64 *to, const struct statx *from)
> -{
> -  /* Clear both pad and reserved fields.  */
> -  memset (to, 0, sizeof (*to));
> -
> -  to->st_dev = ((from->stx_dev_minor & 0xff) | (from->stx_dev_major << 8)
> -		| ((from->stx_dev_minor & ~0xff) << 12));
> -  to->st_ino = from->stx_ino;
> -  to->st_mode = from->stx_mode;
> -  to->st_nlink = from->stx_nlink;
> -  to->st_uid = from->stx_uid;
> -  to->st_gid = from->stx_gid;
> -  to->st_rdev = ((from->stx_rdev_minor & 0xff) | (from->stx_rdev_major << 8)
> -		 | ((from->stx_rdev_minor & ~0xff) << 12));
> -  to->st_size = from->stx_size;
> -  to->st_blksize = from->stx_blksize;
> -  to->st_blocks = from->stx_blocks;
> -
> -  to->st_atime = from->stx_atime.tv_sec;
> -  to->st_atim.tv_nsec = from->stx_atime.tv_nsec;
> -  to->st_mtime = from->stx_mtime.tv_sec;
> -  to->st_mtim.tv_nsec = from->stx_mtime.tv_nsec;
> -  to->st_ctime = from->stx_ctime.tv_sec;
> -  to->st_ctim.tv_nsec = from->stx_ctime.tv_nsec;
> -}
> -#endif
>
Maciej W. Rozycki April 1, 2021, 12:07 a.m. UTC | #2 
On Fri, 19 Mar 2021, Adhemerval Zanella via Libc-alpha wrote:

> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
> index c3a030af58..b9b8cd994b 100644
> --- a/sysdeps/unix/sysv/linux/fstatat64.c
> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
> @@ -117,7 +137,8 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>  }
>  
>  #if (__WORDSIZE == 32 \
> -     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
> +     || defined STAT_HAS_TIME32

 Weird indentation here (which made me wonder how the ordering between && 
and || has been resolved so that GCC does not complain before I realised 
it's simply misindented parenthesisation).  Only caught as I saw the GIT 
commit message though.

  Maciej
Adhemerval Zanella April 1, 2021, 12:45 p.m. UTC | #3 
On 31/03/2021 21:07, Maciej W. Rozycki wrote:
> On Fri, 19 Mar 2021, Adhemerval Zanella via Libc-alpha wrote:
> 
>> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
>> index c3a030af58..b9b8cd994b 100644
>> --- a/sysdeps/unix/sysv/linux/fstatat64.c
>> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
>> @@ -117,7 +137,8 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>>  }
>>  
>>  #if (__WORDSIZE == 32 \
>> -     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
>> +     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
>> +     || defined STAT_HAS_TIME32
> 
>  Weird indentation here (which made me wonder how the ordering between && 
> and || has been resolved so that GCC does not complain before I realised 
> it's simply misindented parenthesisation).  Only caught as I saw the GIT 
> commit message though.

The preprocessor indentation is confusing sometimes, but I think it should
adjusted only slight to follow other usages:

diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
index f968e4ef05..ab25f64187 100644
--- a/sysdeps/unix/sysv/linux/fstatat64.c
+++ b/sysdeps/unix/sysv/linux/fstatat64.c
@@ -137,7 +137,7 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
 
 #if (__WORDSIZE == 32 \
      && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
-     || defined STAT_HAS_TIME32
+    || defined STAT_HAS_TIME32
 # define FSTATAT_USE_STATX 1
 #else

Am I missing something here?
Maciej W. Rozycki April 1, 2021, 6 p.m. UTC | #4 
On Thu, 1 Apr 2021, Adhemerval Zanella via Libc-alpha wrote:

> >  Weird indentation here (which made me wonder how the ordering between && 
> > and || has been resolved so that GCC does not complain before I realised 
> > it's simply misindented parenthesisation).  Only caught as I saw the GIT 
> > commit message though.
> 
> The preprocessor indentation is confusing sometimes, but I think it should
> adjusted only slight to follow other usages:
> 
> diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
> index f968e4ef05..ab25f64187 100644
> --- a/sysdeps/unix/sysv/linux/fstatat64.c
> +++ b/sysdeps/unix/sysv/linux/fstatat64.c
> @@ -137,7 +137,7 @@ fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
>  
>  #if (__WORDSIZE == 32 \
>       && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
> -     || defined STAT_HAS_TIME32
> +    || defined STAT_HAS_TIME32
>  # define FSTATAT_USE_STATX 1
>  #else
> 
> Am I missing something here?

 No, that's what I meant.

 While using outer parentheses seems to be what the GNU Coding Standards 
suggest for Emacs to indent properly, but I'm no Emacs user and we have 
cases with preprocessor statements missing those, and otherwise there is 
no indentation ambiguity here, so I guess this will be OK with your 
amendment above.

  Maciej
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/fstatat64.c b/sysdeps/unix/sysv/linux/fstatat64.c
index c3a030af58..b9b8cd994b 100644
--- a/sysdeps/unix/sysv/linux/fstatat64.c
+++ b/sysdeps/unix/sysv/linux/fstatat64.c
@@ -24,9 +24,9 @@ 
 #include <kernel_stat.h>
 #include <sysdep.h>
 #include <time.h>
-#include <statx_cp.h>
 #include <kstat_cp.h>
 #include <stat_t64_cp.h>
+#include <sys/sysmacros.h>
 
 #if __TIMESIZE == 64 \
      && (__WORDSIZE == 32 \
@@ -50,8 +50,28 @@  fstatat64_time64_statx (int fd, const char *file, struct __stat64_t64 *buf,
   struct statx tmp;
   int r = INTERNAL_SYSCALL_CALL (statx, fd, file, AT_NO_AUTOMOUNT | flag,
 				 STATX_BASIC_STATS, &tmp);
-  if (r == 0)
-    __cp_stat64_t64_statx (buf, &tmp);
+  if (r != 0)
+    return r;
+
+  *buf = (struct __stat64_t64) {
+    .st_dev = makedev (tmp.stx_dev_major, tmp.stx_dev_minor),
+    .st_rdev = makedev (tmp.stx_rdev_major, tmp.stx_rdev_minor),
+    .st_ino = tmp.stx_ino,
+    .st_mode = tmp.stx_mode,
+    .st_nlink = tmp.stx_nlink,
+    .st_uid = tmp.stx_uid,
+    .st_gid = tmp.stx_gid,
+    .st_atime = tmp.stx_atime.tv_sec,
+    .st_atim.tv_nsec = tmp.stx_atime.tv_nsec,
+    .st_mtime = tmp.stx_mtime.tv_sec,
+    .st_mtim.tv_nsec = tmp.stx_mtime.tv_nsec,
+    .st_ctime = tmp.stx_ctime.tv_sec,
+    .st_ctim.tv_nsec = tmp.stx_ctime.tv_nsec,
+    .st_size = tmp.stx_size,
+    .st_blocks = tmp.stx_blocks,
+    .st_blksize = tmp.stx_blksize,
+  };
+
   return r;
 }
 
@@ -117,7 +137,8 @@  fstatat64_time64_stat (int fd, const char *file, struct __stat64_t64 *buf,
 }
 
 #if (__WORDSIZE == 32 \
-     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
+     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) \
+     || defined STAT_HAS_TIME32
 # define FSTATAT_USE_STATX 1
 #else
 # define FSTATAT_USE_STATX 0
diff --git a/sysdeps/unix/sysv/linux/fxstat64.c b/sysdeps/unix/sysv/linux/fxstat64.c
index be12798273..23d9d92b00 100644
--- a/sysdeps/unix/sysv/linux/fxstat64.c
+++ b/sysdeps/unix/sysv/linux/fxstat64.c
@@ -25,6 +25,7 @@ 
 #include <xstatconv.h>
 #include <statx_cp.h>
 #include <shlib-compat.h>
+#include <sys/sysmacros.h>
 
 #if LIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_33)
 
diff --git a/sysdeps/unix/sysv/linux/mips/kernel_stat.h b/sysdeps/unix/sysv/linux/mips/kernel_stat.h
index e4b0f211ca..19524f7ea4 100644
--- a/sysdeps/unix/sysv/linux/mips/kernel_stat.h
+++ b/sysdeps/unix/sysv/linux/mips/kernel_stat.h
@@ -67,5 +67,9 @@  struct kernel_stat
 #else
 # define STATFS_IS_STATFS64 0
 #endif
+/* MIPS64 has unsigned 32 bit timestamps fields, so use statx as well.  */
+#if _MIPS_SIM == _ABI64
+# define STAT_HAS_TIME32
+#endif
 
 #endif
diff --git a/sysdeps/unix/sysv/linux/statx_cp.c b/sysdeps/unix/sysv/linux/statx_cp.c
index 53068704c6..73405a9612 100644
--- a/sysdeps/unix/sysv/linux/statx_cp.c
+++ b/sysdeps/unix/sysv/linux/statx_cp.c
@@ -48,32 +48,3 @@  __cp_stat64_statx (struct stat64 *to, struct statx *from)
 }
 #endif
 
-#if (__WORDSIZE == 32 \
-     && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32))
-void
-__cp_stat64_t64_statx (struct __stat64_t64 *to, const struct statx *from)
-{
-  /* Clear both pad and reserved fields.  */
-  memset (to, 0, sizeof (*to));
-
-  to->st_dev = ((from->stx_dev_minor & 0xff) | (from->stx_dev_major << 8)
-		| ((from->stx_dev_minor & ~0xff) << 12));
-  to->st_ino = from->stx_ino;
-  to->st_mode = from->stx_mode;
-  to->st_nlink = from->stx_nlink;
-  to->st_uid = from->stx_uid;
-  to->st_gid = from->stx_gid;
-  to->st_rdev = ((from->stx_rdev_minor & 0xff) | (from->stx_rdev_major << 8)
-		 | ((from->stx_rdev_minor & ~0xff) << 12));
-  to->st_size = from->stx_size;
-  to->st_blksize = from->stx_blksize;
-  to->st_blocks = from->stx_blocks;
-
-  to->st_atime = from->stx_atime.tv_sec;
-  to->st_atim.tv_nsec = from->stx_atime.tv_nsec;
-  to->st_mtime = from->stx_mtime.tv_sec;
-  to->st_mtim.tv_nsec = from->stx_mtime.tv_nsec;
-  to->st_ctime = from->stx_ctime.tv_sec;
-  to->st_ctim.tv_nsec = from->stx_ctime.tv_nsec;
-}
-#endif