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

Adhemerval Zanella adhemerval.zanella@linaro.org
Thu Dec 10 21:18:06 GMT 2020



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;
> 


More information about the Libc-alpha mailing list