[PATCH v5 1/3] y2038: include: Move struct __timespec64 definition to a separate file

[Adhemerval Zanella]

On 17/03/2020 08:45, Lukasz Majewski wrote:
> The struct __timespec64's definition has been moved from ./include/time.h to
> ./include/struct___timespec64.h.
> 
> This change would prevent from polluting other glibc namespaces (when
> headers are modified to support 64 bit time on architectures with
> __WORDSIZE==32).
> 
> Now it is possible to just include definition of this particular structure
> when needed.
> 
> ---
> Changes for v5:
> - Move struct___timespec64.h from include/bits/types to include
> 
> Changes for v4:
> - None
> 
> Changes for v3:
> - Rebase to the newest glibc's - master
> 
> Changes for v2:
> - New patch - as suggested by Andreas Schwab

LGTM with the following changes.

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

> ---
>  include/struct___timespec64.h | 26 ++++++++++++++++++++++++++
>  include/time.h                | 24 +-----------------------
>  2 files changed, 27 insertions(+), 23 deletions(-)
>  create mode 100644 include/struct___timespec64.h
> 
> diff --git a/include/struct___timespec64.h b/include/struct___timespec64.h
> new file mode 100644
> index 0000000000..0c60f144c8
> --- /dev/null
> +++ b/include/struct___timespec64.h
> @@ -0,0 +1,26 @@
> +#ifndef _TIMESPEC64_H
> +#define _TIMESPEC64_H 1

The include guard name usually try to give some information about the
file naming (in situations where it might clash it might add some
path additional info, such as for sysdep.h).  Also for include guards
extra pre-processor directives does not require to add extra spaces.

So I recommend replace with:

  #ifndef _STRUCT_TIMESPEC64_H
  #define _STRUCT_TIMESPEC64_H

  #if __TIMESIZE == 64
  [...]

(and to indent the following pre-processors accordingly).

> +# if __TIMESIZE == 64
> +#  define __timespec64 timespec
> +# else
> +# include <endian.h>
> +/* The glibc Y2038-proof struct __timespec64 structure for a time value.
> +   To keep things Posix-ish, we keep the nanoseconds field a 32-bit
> +   signed long, but since the Linux field is a 64-bit signed int, we
> +   pad our tv_nsec with a 32-bit unnamed bit-field padding.
> +
> +   As a general rule the Linux kernel is ignoring upper 32 bits of
> +   tv_nsec field.  */
> +struct __timespec64
> +{
> +  __time64_t tv_sec;         /* Seconds */
> +#  if BYTE_ORDER == BIG_ENDIAN
> +  __int32_t :32;             /* Padding */
> +  __int32_t tv_nsec;         /* Nanoseconds */
> +#  else
> +  __int32_t tv_nsec;         /* Nanoseconds */
> +  __int32_t :32;             /* Padding */
> +#  endif
> +};
> +# endif
> +#endif

I would recommend add a comment about for which endif refers to,
using the recommendation above it would be:

  # endif /* _STRUCT_TIMESPEC64_H  */

> diff --git a/include/time.h b/include/time.h
> index c221b68c32..9cf91ce4d6 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -3,11 +3,11 @@
>  
>  #ifndef _ISOMAC
>  # include <bits/types/struct_timeval.h>
> +# include <struct___timespec64.h>
>  # include <bits/types/locale_t.h>
>  # include <stdbool.h>
>  # include <time/mktime-internal.h>
>  # include <sys/time.h>
> -# include <endian.h>
>  # include <time-clockid.h>
>  # include <sys/time.h>
>  
> @@ -61,28 +61,6 @@ extern void __tzset_parse_tz (const char *tz) attribute_hidden;
>  extern void __tz_compute (__time64_t timer, struct tm *tm, int use_localtime)
>    __THROW attribute_hidden;
>  
> -#if __TIMESIZE == 64
> -# define __timespec64 timespec
> -#else
> -/* The glibc Y2038-proof struct __timespec64 structure for a time value.
> -   To keep things Posix-ish, we keep the nanoseconds field a 32-bit
> -   signed long, but since the Linux field is a 64-bit signed int, we
> -   pad our tv_nsec with a 32-bit unnamed bit-field padding.
> -
> -   As a general rule the Linux kernel is ignoring upper 32 bits of
> -   tv_nsec field.  */
> -struct __timespec64
> -{
> -  __time64_t tv_sec;         /* Seconds */
> -# if BYTE_ORDER == BIG_ENDIAN
> -  __int32_t :32;             /* Padding */
> -  __int32_t tv_nsec;         /* Nanoseconds */
> -# else
> -  __int32_t tv_nsec;         /* Nanoseconds */
> -  __int32_t :32;             /* Padding */
> -# endif
> -};
> -#endif
>  
>  #if __TIMESIZE == 64
>  # define __itimerspec64 itimerspec
>