[PATCH] alpha: Remove anonymous union in struct stat [BZ #27042]

[PATCH] alpha: Remove anonymous union in struct stat [BZ #27042]

[Matt Turner]

This is clever, but it confuses downstream detection in at least zstd
and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
which is not provided by the path using the anonymous union; glib checks
for the presence of 'st_mtimensec' in struct stat but then tries to
access that field in struct statx (which might be a bug on its own).
---
 .../unix/sysv/linux/alpha/bits/struct_stat.h  | 23 ++++---------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h b/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
index 1c9b4248b8..dddc5fb5a0 100644
--- a/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
+++ b/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
@@ -27,26 +27,13 @@
    'struct timespec'.  This is the type used whenever possible but the
    Unix namespace rules do not allow the identifier 'timespec' to appear
    in the <sys/stat.h> header.  Therefore we have to handle the use of
-   this header in strictly standard-compliant sources special.
-
-   Use neat tidy anonymous unions and structures when possible.  */
+   this header in strictly standard-compliant sources special.  */
 
 #ifdef __USE_XOPEN2K8
-# if __GNUC_PREREQ(3,3)
-#  define __ST_TIME(X)				\
-	__extension__ union {			\
-	    struct timespec st_##X##tim;	\
-	    struct {				\
-		__time_t st_##X##time;		\
-		unsigned long st_##X##timensec;	\
-	    };					\
-	}
-# else
-#  define __ST_TIME(X) struct timespec st_##X##tim
-#  define st_atime st_atim.tv_sec
-#  define st_mtime st_mtim.tv_sec
-#  define st_ctime st_ctim.tv_sec
-# endif
+# define __ST_TIME(X) struct timespec st_##X##tim
+# define st_atime st_atim.tv_sec
+# define st_mtime st_mtim.tv_sec
+# define st_ctime st_ctim.tv_sec
 #else
 # define __ST_TIME(X)				\
 	__time_t st_##X##time;			\
-- 
2.26.2

Re: [PATCH] alpha: Remove anonymous union in struct stat [BZ #27042]

[Florian Weimer]

* Matt Turner via Libc-alpha:

> This is clever, but it confuses downstream detection in at least zstd
> and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
> which is not provided by the path using the anonymous union; glib checks
> for the presence of 'st_mtimensec' in struct stat but then tries to
> access that field in struct statx (which might be a bug on its own).

A more conservative approach would be adding

#define st_atime st_atime
#define st_ctime st_ctime
#define st_mtime st_mtime

to the anonymous union case.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

[PATCHv2] alpha: Remove anonymous union in struct stat [BZ #27042]

[Matt Turner]

This is clever, but it confuses downstream detection in at least zstd
and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
which is not provided by the path using the anonymous union; glib checks
for the presence of 'st_mtimensec' in struct stat but then tries to
access that field in struct statx (which might be a bug on its own).
---
The macros affect the definitions in kernel_stat.h, so rename them. Not
thrilled.

 .../unix/sysv/linux/alpha/bits/struct_stat.h  | 81 ++++++++++---------
 sysdeps/unix/sysv/linux/alpha/kernel_stat.h   | 24 +++---
 sysdeps/unix/sysv/linux/alpha/xstatconv.c     | 24 +++---
 3 files changed, 66 insertions(+), 63 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h b/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
index 1c9b4248b8..d2aae9fdd7 100644
--- a/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
+++ b/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
@@ -23,37 +23,6 @@
 #ifndef _BITS_STRUCT_STAT_H
 #define _BITS_STRUCT_STAT_H	1
 
-/* Nanosecond resolution timestamps are stored in a format equivalent to
-   'struct timespec'.  This is the type used whenever possible but the
-   Unix namespace rules do not allow the identifier 'timespec' to appear
-   in the <sys/stat.h> header.  Therefore we have to handle the use of
-   this header in strictly standard-compliant sources special.
-
-   Use neat tidy anonymous unions and structures when possible.  */
-
-#ifdef __USE_XOPEN2K8
-# if __GNUC_PREREQ(3,3)
-#  define __ST_TIME(X)				\
-	__extension__ union {			\
-	    struct timespec st_##X##tim;	\
-	    struct {				\
-		__time_t st_##X##time;		\
-		unsigned long st_##X##timensec;	\
-	    };					\
-	}
-# else
-#  define __ST_TIME(X) struct timespec st_##X##tim
-#  define st_atime st_atim.tv_sec
-#  define st_mtime st_mtim.tv_sec
-#  define st_ctime st_ctim.tv_sec
-# endif
-#else
-# define __ST_TIME(X)				\
-	__time_t st_##X##time;			\
-	unsigned long st_##X##timensec
-#endif
-
-
 struct stat
   {
     __dev_t st_dev;		/* Device.  */
@@ -77,9 +46,27 @@ struct stat
     __blksize_t st_blksize;	/* Optimal block size for I/O.  */
     __nlink_t st_nlink;		/* Link count.  */
     int __pad2;			/* Real padding.  */
-    __ST_TIME(a);		/* Time of last access.  */
-    __ST_TIME(m);		/* Time of last modification.  */
-    __ST_TIME(c);		/* Time of last status change.  */
+#ifdef __USE_XOPEN2K8
+    /* Nanosecond resolution timestamps are stored in a format
+       equivalent to 'struct timespec'.  This is the type used
+       whenever possible but the Unix namespace rules do not allow the
+       identifier 'timespec' to appear in the <sys/stat.h> header.
+       Therefore we have to handle the use of this header in strictly
+       standard-compliant sources special.  */
+    struct timespec st_atim;		/* Time of last access.  */
+    struct timespec st_mtim;		/* Time of last modification.  */
+    struct timespec st_ctim;		/* Time of last status change.  */
+# define st_atime st_atim.tv_sec	/* Backward compatibility.  */
+# define st_mtime st_mtim.tv_sec
+# define st_ctime st_ctim.tv_sec
+#else
+    __time_t st_atime;			/* Time of last access.  */
+    unsigned long int st_atimensec;	/* Nscecs of last access.  */
+    __time_t st_mtime;			/* Time of last modification.  */
+    unsigned long int st_mtimensec;	/* Nsecs of last modification.  */
+    __time_t st_ctime;			/* Time of last status change.  */
+    unsigned long int st_ctimensec;	/* Nsecs of last status change.  */
+#endif
     long __glibc_reserved[3];
   };
 
@@ -98,15 +85,31 @@ struct stat64
     __blksize_t st_blksize;	/* Optimal block size for I/O.  */
     __nlink_t st_nlink;		/* Link count.  */
     int __pad0;			/* Real padding.  */
-    __ST_TIME(a);		/* Time of last access.  */
-    __ST_TIME(m);		/* Time of last modification.  */
-    __ST_TIME(c);		/* Time of last status change.  */
+#ifdef __USE_XOPEN2K8
+    /* Nanosecond resolution timestamps are stored in a format
+       equivalent to 'struct timespec'.  This is the type used
+       whenever possible but the Unix namespace rules do not allow the
+       identifier 'timespec' to appear in the <sys/stat.h> header.
+       Therefore we have to handle the use of this header in strictly
+       standard-compliant sources special.  */
+    struct timespec st_atim;		/* Time of last access.  */
+    struct timespec st_mtim;		/* Time of last modification.  */
+    struct timespec st_ctim;		/* Time of last status change.  */
+# define st_atime st_atim.tv_sec	/* Backward compatibility.  */
+# define st_mtime st_mtim.tv_sec
+# define st_ctime st_ctim.tv_sec
+#else
+    __time_t st_atime;			/* Time of last access.  */
+    unsigned long int st_atimensec;	/* Nscecs of last access.  */
+    __time_t st_mtime;			/* Time of last modification.  */
+    unsigned long int st_mtimensec;	/* Nsecs of last modification.  */
+    __time_t st_ctime;			/* Time of last status change.  */
+    unsigned long int st_ctimensec;	/* Nsecs of last status change.  */
+#endif
     long __glibc_reserved[3];
   };
 #endif
 
-#undef __ST_TIME
-
 /* Tell code we have these members.  */
 #define	_STATBUF_ST_BLKSIZE
 #define _STATBUF_ST_RDEV
diff --git a/sysdeps/unix/sysv/linux/alpha/kernel_stat.h b/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
index ff69045f8f..a292920969 100644
--- a/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
+++ b/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
@@ -9,9 +9,9 @@ struct kernel_stat
     unsigned int st_gid;
     unsigned int st_rdev;
     long int st_size;
-    unsigned long int st_atime;
-    unsigned long int st_mtime;
-    unsigned long int st_ctime;
+    unsigned long int st_atime_sec;
+    unsigned long int st_mtime_sec;
+    unsigned long int st_ctime_sec;
     unsigned int st_blksize;
     int st_blocks;
     unsigned int st_flags;
@@ -34,11 +34,11 @@ struct kernel_stat64
     unsigned int    st_nlink;
     unsigned int    __pad0;
 
-    unsigned long   st_atime;
+    unsigned long   st_atime_sec;
     unsigned long   st_atimensec;
-    unsigned long   st_mtime;
+    unsigned long   st_mtime_sec;
     unsigned long   st_mtimensec;
-    unsigned long   st_ctime;
+    unsigned long   st_ctime_sec;
     unsigned long   st_ctimensec;
     long            __glibc_reserved[3];
   };
@@ -54,9 +54,9 @@ struct glibc2_stat
     __gid_t st_gid;
     __dev_t st_rdev;
     __off_t st_size;
-    __time_t st_atime;
-    __time_t st_mtime;
-    __time_t st_ctime;
+    __time_t st_atime_sec;
+    __time_t st_mtime_sec;
+    __time_t st_ctime_sec;
     unsigned int st_blksize;
     int st_blocks;
     unsigned int st_flags;
@@ -74,9 +74,9 @@ struct glibc21_stat
     __gid_t st_gid;
     __dev_t st_rdev;
     __off_t st_size;
-    __time_t st_atime;
-    __time_t st_mtime;
-    __time_t st_ctime;
+    __time_t st_atime_sec;
+    __time_t st_mtime_sec;
+    __time_t st_ctime_sec;
     __blkcnt64_t st_blocks;
     __blksize_t st_blksize;
     unsigned int st_flags;
diff --git a/sysdeps/unix/sysv/linux/alpha/xstatconv.c b/sysdeps/unix/sysv/linux/alpha/xstatconv.c
index f716a10f34..43224a7f25 100644
--- a/sysdeps/unix/sysv/linux/alpha/xstatconv.c
+++ b/sysdeps/unix/sysv/linux/alpha/xstatconv.c
@@ -44,9 +44,9 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
 	buf->st_gid = kbuf->st_gid;
 	buf->st_rdev = kbuf->st_rdev;
 	buf->st_size = kbuf->st_size;
-	buf->st_atime = kbuf->st_atime;
-	buf->st_mtime = kbuf->st_mtime;
-	buf->st_ctime = kbuf->st_ctime;
+	buf->st_atime_sec = kbuf->st_atime_sec;
+	buf->st_mtime_sec = kbuf->st_mtime_sec;
+	buf->st_ctime_sec = kbuf->st_ctime_sec;
 	buf->st_blksize = kbuf->st_blksize;
 	buf->st_blocks = kbuf->st_blocks;
 	buf->st_flags = kbuf->st_flags;
@@ -66,9 +66,9 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
 	buf->st_gid = kbuf->st_gid;
 	buf->st_rdev = kbuf->st_rdev;
 	buf->st_size = kbuf->st_size;
-	buf->st_atime = kbuf->st_atime;
-	buf->st_mtime = kbuf->st_mtime;
-	buf->st_ctime = kbuf->st_ctime;
+	buf->st_atime_sec = kbuf->st_atime_sec;
+	buf->st_mtime_sec = kbuf->st_mtime_sec;
+	buf->st_ctime_sec = kbuf->st_ctime_sec;
 	buf->st_blocks = kbuf->st_blocks;
 	buf->st_blksize = kbuf->st_blksize;
 	buf->st_flags = kbuf->st_flags;
@@ -98,12 +98,12 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
 	buf->st_nlink = kbuf->st_nlink;
 	buf->__pad0 = 0;
 
-	buf->st_atime = kbuf->st_atime;
-	buf->st_atimensec = 0;
-	buf->st_mtime = kbuf->st_mtime;
-	buf->st_mtimensec = 0;
-	buf->st_ctime = kbuf->st_ctime;
-	buf->st_ctimensec = 0;
+	buf->st_atim.tv_sec = kbuf->st_atime_sec;
+	buf->st_atim.tv_nsec = 0;
+	buf->st_mtim.tv_sec = kbuf->st_mtime_sec;
+	buf->st_mtim.tv_nsec = 0;
+	buf->st_ctim.tv_sec = kbuf->st_ctime_sec;
+	buf->st_ctim.tv_nsec = 0;
 
 	buf->__glibc_reserved[0] = 0;
 	buf->__glibc_reserved[1] = 0;
-- 
2.26.2

Re: [PATCH] alpha: Remove anonymous union in struct stat [BZ #27042]

[Matt Turner]

On Wed, Dec 9, 2020 at 12:11 PM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Matt Turner via Libc-alpha:
>
> > This is clever, but it confuses downstream detection in at least zstd
> > and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
> > which is not provided by the path using the anonymous union; glib checks
> > for the presence of 'st_mtimensec' in struct stat but then tries to
> > access that field in struct statx (which might be a bug on its own).
>
> A more conservative approach would be adding
>
> #define st_atime st_atime
> #define st_ctime st_ctime
> #define st_mtime st_mtime
>
> to the anonymous union case.

Thanks. That works for zstd, but not for glib—which might be a bug in
glib, like I mentioned. I'll investigate that independently.

Andreas immediately closed the bug I filed with the message

> The existence of the st_?time macros is an implementation detail.

and I have no doubt that he's correct, but it appears that all or
basically all of the architectures' headers provide these macros so
I'd prefer to bring alpha in line rather than convince multiple
projects that their code is technically wrong in a way that only
manifests on an architecture they've never heard of.

v2 sent that deals with the kernel_stat.h interactions.

Re: [PATCHv2] alpha: Remove anonymous union in struct stat [BZ #27042]

[Adhemerval Zanella]

On 09/12/2020 22:19, Matt Turner via Libc-alpha wrote:
> This is clever, but it confuses downstream detection in at least zstd
> and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
> which is not provided by the path using the anonymous union; glib checks
> for the presence of 'st_mtimensec' in struct stat but then tries to
> access that field in struct statx (which might be a bug on its own).
> ---
> The macros affect the definitions in kernel_stat.h, so rename them. Not
> thrilled.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
>  .../unix/sysv/linux/alpha/bits/struct_stat.h  | 81 ++++++++++---------
>  sysdeps/unix/sysv/linux/alpha/kernel_stat.h   | 24 +++---
>  sysdeps/unix/sysv/linux/alpha/xstatconv.c     | 24 +++---
>  3 files changed, 66 insertions(+), 63 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h b/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
> index 1c9b4248b8..d2aae9fdd7 100644
> --- a/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
> +++ b/sysdeps/unix/sysv/linux/alpha/bits/struct_stat.h
> @@ -23,37 +23,6 @@
>  #ifndef _BITS_STRUCT_STAT_H
>  #define _BITS_STRUCT_STAT_H	1
>  
> -/* Nanosecond resolution timestamps are stored in a format equivalent to
> -   'struct timespec'.  This is the type used whenever possible but the
> -   Unix namespace rules do not allow the identifier 'timespec' to appear
> -   in the <sys/stat.h> header.  Therefore we have to handle the use of
> -   this header in strictly standard-compliant sources special.
> -
> -   Use neat tidy anonymous unions and structures when possible.  */
> -
> -#ifdef __USE_XOPEN2K8
> -# if __GNUC_PREREQ(3,3)
> -#  define __ST_TIME(X)				\
> -	__extension__ union {			\
> -	    struct timespec st_##X##tim;	\
> -	    struct {				\
> -		__time_t st_##X##time;		\
> -		unsigned long st_##X##timensec;	\
> -	    };					\
> -	}
> -# else
> -#  define __ST_TIME(X) struct timespec st_##X##tim
> -#  define st_atime st_atim.tv_sec
> -#  define st_mtime st_mtim.tv_sec
> -#  define st_ctime st_ctim.tv_sec
> -# endif
> -#else
> -# define __ST_TIME(X)				\
> -	__time_t st_##X##time;			\
> -	unsigned long st_##X##timensec
> -#endif
> -
> -
>  struct stat
>    {
>      __dev_t st_dev;		/* Device.  */
> @@ -77,9 +46,27 @@ struct stat
>      __blksize_t st_blksize;	/* Optimal block size for I/O.  */
>      __nlink_t st_nlink;		/* Link count.  */
>      int __pad2;			/* Real padding.  */
> -    __ST_TIME(a);		/* Time of last access.  */
> -    __ST_TIME(m);		/* Time of last modification.  */
> -    __ST_TIME(c);		/* Time of last status change.  */
> +#ifdef __USE_XOPEN2K8
> +    /* Nanosecond resolution timestamps are stored in a format
> +       equivalent to 'struct timespec'.  This is the type used
> +       whenever possible but the Unix namespace rules do not allow the
> +       identifier 'timespec' to appear in the <sys/stat.h> header.
> +       Therefore we have to handle the use of this header in strictly
> +       standard-compliant sources special.  */
> +    struct timespec st_atim;		/* Time of last access.  */
> +    struct timespec st_mtim;		/* Time of last modification.  */
> +    struct timespec st_ctim;		/* Time of last status change.  */
> +# define st_atime st_atim.tv_sec	/* Backward compatibility.  */
> +# define st_mtime st_mtim.tv_sec
> +# define st_ctime st_ctim.tv_sec
> +#else
> +    __time_t st_atime;			/* Time of last access.  */
> +    unsigned long int st_atimensec;	/* Nscecs of last access.  */
> +    __time_t st_mtime;			/* Time of last modification.  */
> +    unsigned long int st_mtimensec;	/* Nsecs of last modification.  */
> +    __time_t st_ctime;			/* Time of last status change.  */
> +    unsigned long int st_ctimensec;	/* Nsecs of last status change.  */
> +#endif
>      long __glibc_reserved[3];
>    };
>  
> @@ -98,15 +85,31 @@ struct stat64
>      __blksize_t st_blksize;	/* Optimal block size for I/O.  */
>      __nlink_t st_nlink;		/* Link count.  */
>      int __pad0;			/* Real padding.  */
> -    __ST_TIME(a);		/* Time of last access.  */
> -    __ST_TIME(m);		/* Time of last modification.  */
> -    __ST_TIME(c);		/* Time of last status change.  */
> +#ifdef __USE_XOPEN2K8
> +    /* Nanosecond resolution timestamps are stored in a format
> +       equivalent to 'struct timespec'.  This is the type used
> +       whenever possible but the Unix namespace rules do not allow the
> +       identifier 'timespec' to appear in the <sys/stat.h> header.
> +       Therefore we have to handle the use of this header in strictly
> +       standard-compliant sources special.  */
> +    struct timespec st_atim;		/* Time of last access.  */
> +    struct timespec st_mtim;		/* Time of last modification.  */
> +    struct timespec st_ctim;		/* Time of last status change.  */
> +# define st_atime st_atim.tv_sec	/* Backward compatibility.  */
> +# define st_mtime st_mtim.tv_sec
> +# define st_ctime st_ctim.tv_sec
> +#else
> +    __time_t st_atime;			/* Time of last access.  */
> +    unsigned long int st_atimensec;	/* Nscecs of last access.  */
> +    __time_t st_mtime;			/* Time of last modification.  */
> +    unsigned long int st_mtimensec;	/* Nsecs of last modification.  */
> +    __time_t st_ctime;			/* Time of last status change.  */
> +    unsigned long int st_ctimensec;	/* Nsecs of last status change.  */
> +#endif
>      long __glibc_reserved[3];
>    };
>  #endif
>  
> -#undef __ST_TIME
> -
>  /* Tell code we have these members.  */
>  #define	_STATBUF_ST_BLKSIZE
>  #define _STATBUF_ST_RDEV
> diff --git a/sysdeps/unix/sysv/linux/alpha/kernel_stat.h b/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
> index ff69045f8f..a292920969 100644
> --- a/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
> +++ b/sysdeps/unix/sysv/linux/alpha/kernel_stat.h
> @@ -9,9 +9,9 @@ struct kernel_stat
>      unsigned int st_gid;
>      unsigned int st_rdev;
>      long int st_size;
> -    unsigned long int st_atime;
> -    unsigned long int st_mtime;
> -    unsigned long int st_ctime;
> +    unsigned long int st_atime_sec;
> +    unsigned long int st_mtime_sec;
> +    unsigned long int st_ctime_sec;
>      unsigned int st_blksize;
>      int st_blocks;
>      unsigned int st_flags;
> @@ -34,11 +34,11 @@ struct kernel_stat64
>      unsigned int    st_nlink;
>      unsigned int    __pad0;
>  
> -    unsigned long   st_atime;
> +    unsigned long   st_atime_sec;
>      unsigned long   st_atimensec;
> -    unsigned long   st_mtime;
> +    unsigned long   st_mtime_sec;
>      unsigned long   st_mtimensec;
> -    unsigned long   st_ctime;
> +    unsigned long   st_ctime_sec;
>      unsigned long   st_ctimensec;
>      long            __glibc_reserved[3];
>    };
> @@ -54,9 +54,9 @@ struct glibc2_stat
>      __gid_t st_gid;
>      __dev_t st_rdev;
>      __off_t st_size;
> -    __time_t st_atime;
> -    __time_t st_mtime;
> -    __time_t st_ctime;
> +    __time_t st_atime_sec;
> +    __time_t st_mtime_sec;
> +    __time_t st_ctime_sec;
>      unsigned int st_blksize;
>      int st_blocks;
>      unsigned int st_flags;
> @@ -74,9 +74,9 @@ struct glibc21_stat
>      __gid_t st_gid;
>      __dev_t st_rdev;
>      __off_t st_size;
> -    __time_t st_atime;
> -    __time_t st_mtime;
> -    __time_t st_ctime;
> +    __time_t st_atime_sec;
> +    __time_t st_mtime_sec;
> +    __time_t st_ctime_sec;
>      __blkcnt64_t st_blocks;
>      __blksize_t st_blksize;
>      unsigned int st_flags;
> diff --git a/sysdeps/unix/sysv/linux/alpha/xstatconv.c b/sysdeps/unix/sysv/linux/alpha/xstatconv.c
> index f716a10f34..43224a7f25 100644
> --- a/sysdeps/unix/sysv/linux/alpha/xstatconv.c
> +++ b/sysdeps/unix/sysv/linux/alpha/xstatconv.c
> @@ -44,9 +44,9 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
>  	buf->st_gid = kbuf->st_gid;
>  	buf->st_rdev = kbuf->st_rdev;
>  	buf->st_size = kbuf->st_size;
> -	buf->st_atime = kbuf->st_atime;
> -	buf->st_mtime = kbuf->st_mtime;
> -	buf->st_ctime = kbuf->st_ctime;
> +	buf->st_atime_sec = kbuf->st_atime_sec;
> +	buf->st_mtime_sec = kbuf->st_mtime_sec;
> +	buf->st_ctime_sec = kbuf->st_ctime_sec;
>  	buf->st_blksize = kbuf->st_blksize;
>  	buf->st_blocks = kbuf->st_blocks;
>  	buf->st_flags = kbuf->st_flags;
> @@ -66,9 +66,9 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
>  	buf->st_gid = kbuf->st_gid;
>  	buf->st_rdev = kbuf->st_rdev;
>  	buf->st_size = kbuf->st_size;
> -	buf->st_atime = kbuf->st_atime;
> -	buf->st_mtime = kbuf->st_mtime;
> -	buf->st_ctime = kbuf->st_ctime;
> +	buf->st_atime_sec = kbuf->st_atime_sec;
> +	buf->st_mtime_sec = kbuf->st_mtime_sec;
> +	buf->st_ctime_sec = kbuf->st_ctime_sec;
>  	buf->st_blocks = kbuf->st_blocks;
>  	buf->st_blksize = kbuf->st_blksize;
>  	buf->st_flags = kbuf->st_flags;
> @@ -98,12 +98,12 @@ __xstat_conv (int vers, struct kernel_stat *kbuf, void *ubuf)
>  	buf->st_nlink = kbuf->st_nlink;
>  	buf->__pad0 = 0;
>  
> -	buf->st_atime = kbuf->st_atime;
> -	buf->st_atimensec = 0;
> -	buf->st_mtime = kbuf->st_mtime;
> -	buf->st_mtimensec = 0;
> -	buf->st_ctime = kbuf->st_ctime;
> -	buf->st_ctimensec = 0;
> +	buf->st_atim.tv_sec = kbuf->st_atime_sec;
> +	buf->st_atim.tv_nsec = 0;
> +	buf->st_mtim.tv_sec = kbuf->st_mtime_sec;
> +	buf->st_mtim.tv_nsec = 0;
> +	buf->st_ctim.tv_sec = kbuf->st_ctime_sec;
> +	buf->st_ctim.tv_nsec = 0;
>  
>  	buf->__glibc_reserved[0] = 0;
>  	buf->__glibc_reserved[1] = 0;
> 

Re: [PATCHv2] alpha: Remove anonymous union in struct stat [BZ #27042]

[Matt Turner]

On Thu, Dec 10, 2020 at 4:18 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
> On 09/12/2020 22:19, Matt Turner via Libc-alpha wrote:
> > This is clever, but it confuses downstream detection in at least zstd
> > and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
> > which is not provided by the path using the anonymous union; glib checks
> > for the presence of 'st_mtimensec' in struct stat but then tries to
> > access that field in struct statx (which might be a bug on its own).
> > ---
> > The macros affect the definitions in kernel_stat.h, so rename them. Not
> > thrilled.
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

Thank you. Should we wait for any other input?

Re: [PATCHv2] alpha: Remove anonymous union in struct stat [BZ #27042]

[Adhemerval Zanella]

On 15/12/2020 13:43, Matt Turner wrote:
> On Thu, Dec 10, 2020 at 4:18 PM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>> On 09/12/2020 22:19, Matt Turner via Libc-alpha wrote:
>>> This is clever, but it confuses downstream detection in at least zstd
>>> and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
>>> which is not provided by the path using the anonymous union; glib checks
>>> for the presence of 'st_mtimensec' in struct stat but then tries to
>>> access that field in struct statx (which might be a bug on its own).
>>> ---
>>> The macros affect the definitions in kernel_stat.h, so rename them. Not
>>> thrilled.
>>
>> LGTM, thanks.
>>
>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> 
> Thank you. Should we wait for any other input?

I think you are already following Florian suggestion and it align
of what other ABIs are already doing. 

Re: [PATCHv2] alpha: Remove anonymous union in struct stat [BZ #27042]

[Florian Weimer]

* Matt Turner via Libc-alpha:

> On Thu, Dec 10, 2020 at 4:18 PM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>> On 09/12/2020 22:19, Matt Turner via Libc-alpha wrote:
>> > This is clever, but it confuses downstream detection in at least zstd
>> > and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
>> > which is not provided by the path using the anonymous union; glib checks
>> > for the presence of 'st_mtimensec' in struct stat but then tries to
>> > access that field in struct statx (which might be a bug on its own).
>> > ---
>> > The macros affect the definitions in kernel_stat.h, so rename them. Not
>> > thrilled.
>>
>> LGTM, thanks.
>>
>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
> Thank you. Should we wait for any other input?

I think Adhemerval's review is sufficient.  Thanks.

Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: [PATCHv2] alpha: Remove anonymous union in struct stat [BZ #27042]

[Matt Turner]

On Tue, Dec 15, 2020 at 11:56 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Matt Turner via Libc-alpha:
>
> > On Thu, Dec 10, 2020 at 4:18 PM Adhemerval Zanella via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >> On 09/12/2020 22:19, Matt Turner via Libc-alpha wrote:
> >> > This is clever, but it confuses downstream detection in at least zstd
> >> > and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
> >> > which is not provided by the path using the anonymous union; glib checks
> >> > for the presence of 'st_mtimensec' in struct stat but then tries to
> >> > access that field in struct statx (which might be a bug on its own).
> >> > ---
> >> > The macros affect the definitions in kernel_stat.h, so rename them. Not
> >> > thrilled.
> >>
> >> LGTM, thanks.
> >>
> >> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> >
> > Thank you. Should we wait for any other input?
>
> I think Adhemerval's review is sufficient.  Thanks.

Thank you both. Could I bother one of you to push the patch?

Re: [PATCHv2] alpha: Remove anonymous union in struct stat [BZ #27042]

[Adhemerval Zanella]

On 17/12/2020 15:18, Matt Turner wrote:
> On Tue, Dec 15, 2020 at 11:56 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Matt Turner via Libc-alpha:
>>
>>> On Thu, Dec 10, 2020 at 4:18 PM Adhemerval Zanella via Libc-alpha
>>> <libc-alpha@sourceware.org> wrote:
>>>> On 09/12/2020 22:19, Matt Turner via Libc-alpha wrote:
>>>>> This is clever, but it confuses downstream detection in at least zstd
>>>>> and GNOME's glib. zstd has preprocessor tests for the 'st_mtime' macro,
>>>>> which is not provided by the path using the anonymous union; glib checks
>>>>> for the presence of 'st_mtimensec' in struct stat but then tries to
>>>>> access that field in struct statx (which might be a bug on its own).
>>>>> ---
>>>>> The macros affect the definitions in kernel_stat.h, so rename them. Not
>>>>> thrilled.
>>>>
>>>> LGTM, thanks.
>>>>
>>>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>>>
>>> Thank you. Should we wait for any other input?
>>
>> I think Adhemerval's review is sufficient.  Thanks.
> 
> Thank you both. Could I bother one of you to push the patch?
> 

I will handle it.