Re: [PATCH] Make st_*tim visible in stat for POSIX.1-2008

[Corinna Vinschen <vinschen at redhat dot com>]

On Aug 14 12:49, Dionna Amalie Glaze via newlib wrote:
> Fixed the __MISC_VISIBLE part.
> I'm not sure I understand your question. I'm just restructuring how that
> code gets exposed. Where previously the timespec and blocks were defined if
> rtems, and otherwise just the timespec if srv4 etc, I've changed the
> timespec declarations to all be grouped together. The block declarations
> are separate because only the #else after defined(__rtems__) is evaluated
> false and `defined(__svr4__) && !defined(__PPC__) && !defined(__sun__)` is
> evaluated false.
> 
> ---
>  newlib/libc/include/sys/stat.h | 64 ++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 30 deletions(-)
> 
> diff --git a/newlib/libc/include/sys/stat.h b/newlib/libc/include/sys/stat.h
> index eee98db64..d7d08e830 100644
> --- a/newlib/libc/include/sys/stat.h
> +++ b/newlib/libc/include/sys/stat.h
> @@ -24,7 +24,7 @@ extern "C" {
>  #define stat64 stat
>  #endif
>  #else
> -struct stat
> +struct stat
>  {
>    dev_t                st_dev;
>    ino_t                st_ino;
> @@ -34,15 +34,11 @@ struct      stat
>    gid_t                st_gid;
>    dev_t                st_rdev;
>    off_t                st_size;
> -#if defined(__rtems__)
> +#if __MISC_VISIBLE || __POSIX_VISIBLE >= 200809 || defined(__rtems__)

Do we really need that?  I'm cringing at the idea to redefine a struct
based on macros set depending on user settings.  Can't we simplify this?
AFAICS, the timestamps definition of rtems is equivalent to the timestamps
definition of all other targets, except svr4 etc.  The only difference
is the additional st_spare4.

I'd like to make the following suggestion, so all targets except svr4 etc.
default to the POSIX compatible definition:

diff --git a/newlib/libc/include/sys/stat.h b/newlib/libc/include/sys/stat.h
index eee98db64a9a..e460c69c963f 100644
--- a/newlib/libc/include/sys/stat.h
+++ b/newlib/libc/include/sys/stat.h
@@ -34,27 +34,17 @@ struct	stat
   gid_t		st_gid;
   dev_t		st_rdev;
   off_t		st_size;
-#if defined(__rtems__)
-  struct timespec st_atim;
-  struct timespec st_mtim;
-  struct timespec st_ctim;
-  blksize_t     st_blksize;
-  blkcnt_t	st_blocks;
-#else
-  /* SysV/sco doesn't have the rest... But Solaris, eabi does.  */
 #if defined(__svr4__) && !defined(__PPC__) && !defined(__sun__)
   time_t	st_atime;
   time_t	st_mtime;
   time_t	st_ctime;
 #else
-  time_t	st_atime;
-  long		st_spare1;
-  time_t	st_mtime;
-  long		st_spare2;
-  time_t	st_ctime;
-  long		st_spare3;
-  blksize_t	st_blksize;
+  struct timespec st_atim;
+  struct timespec st_mtim;
+  struct timespec st_ctim;
+  blksize_t     st_blksize;
   blkcnt_t	st_blocks;
+#if !defined(__rtems__)
   long	st_spare4[2];
 #endif
 #endif


Thoughts?
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: signature.asc
Description: PGP signature