diff mbox

[2/2] time: Add padding for the timespec if required

Message ID 20190925002903.15928-2-alistair.francis@wdc.com
State New, archived
Headers

Commit Message

Alistair Francis Sept. 25, 2019, 12:29 a.m. UTC 
  If we are running on a 32-bit system with a 64-bit time_t we need to
ensure there is padding around the tv_nsec variable. This is requried as
the timespec is #defined to the __timespec64 struct.

2019-09-20  Alistair Francis  <alistair.francis@wdc.com>

	* time/bits/types/struct_timespec.h: Add padding for the timespec if
	required.
---
This change was tested by running user space tests on RV32.

 time/bits/types/struct_timespec.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Joseph Myers Sept. 25, 2019, 12:57 a.m. UTC | #1 
On Tue, 24 Sep 2019, Alistair Francis wrote:

> +#if __WORDSIZE == 64 \
> +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) || \
> +  __TIMESIZE == 32

|| goes at start of line, not at end of line.

>    __syscall_slong_t tv_nsec;	/* Nanoseconds.  */
> +#else
> +# if __BYTE_ORDER == __BIG_ENDIAN
> +  int: 32;           /* Padding */
> +  long int tv_nsec;  /* Nanoseconds */
> +# else
> +  long int tv_nsec;  /* Nanoseconds */
> +  int: 32;           /* Padding */
> +# endif
> +#endif

The comment formatting in the other cases, with ".  " ('.' and two spaces) 
at end of comment, is correct GNU style.

The patch is OK with those fixes once the <bits/endian.h> changes are 
reviewed and checked in.
Alistair Francis Sept. 25, 2019, 12:57 a.m. UTC | #2 
On Tue, Sep 24, 2019 at 5:57 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Tue, 24 Sep 2019, Alistair Francis wrote:
>
> > +#if __WORDSIZE == 64 \
> > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) || \
> > +  __TIMESIZE == 32
>
> || goes at start of line, not at end of line.
>
> >    __syscall_slong_t tv_nsec; /* Nanoseconds.  */
> > +#else
> > +# if __BYTE_ORDER == __BIG_ENDIAN
> > +  int: 32;           /* Padding */
> > +  long int tv_nsec;  /* Nanoseconds */
> > +# else
> > +  long int tv_nsec;  /* Nanoseconds */
> > +  int: 32;           /* Padding */
> > +# endif
> > +#endif
>
> The comment formatting in the other cases, with ".  " ('.' and two spaces)
> at end of comment, is correct GNU style.

I have fixed all of there.

>
> The patch is OK with those fixes once the <bits/endian.h> changes are
> reviewed and checked in.

Thanks! :)

I don't add a Reviewed-by tag do I?

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com
Lukasz Majewski Sept. 25, 2019, 9:50 a.m. UTC | #3 
Hi Alistair, Joseph,

> On Tue, Sep 24, 2019 at 5:57 PM Joseph Myers
> <joseph@codesourcery.com> wrote:
> >
> > On Tue, 24 Sep 2019, Alistair Francis wrote:
> >  
> > > +#if __WORDSIZE == 64 \
> > > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) ||
> > > \
> > > +  __TIMESIZE == 32  
         ^^^^^^^^^^^^^^^^^ - this may be a bit problematic as 32 bit
         systems (with __TIMESIZE == 32) gain support for 64 bit time
         (so the Y2038 problem is solved).

	 Those systems also would require padding (the code below).

	 However, I do guess that allowing such systems to have the
	 extra padding after this patch (by removing __TIMESIZE==32
	 condition) is not an option for now (and shall be done in the
	 same commit which introduces -D_TIME_BITS=64 support to glibc)
	 ?

> >
> > || goes at start of line, not at end of line.
> >  
> > >    __syscall_slong_t tv_nsec; /* Nanoseconds.  */
> > > +#else
> > > +# if __BYTE_ORDER == __BIG_ENDIAN
> > > +  int: 32;           /* Padding */
> > > +  long int tv_nsec;  /* Nanoseconds */
> > > +# else
> > > +  long int tv_nsec;  /* Nanoseconds */
> > > +  int: 32;           /* Padding */
> > > +# endif
> > > +#endif  
> >
> > The comment formatting in the other cases, with ".  " ('.' and two
> > spaces) at end of comment, is correct GNU style.  
> 
> I have fixed all of there.
> 
> >
> > The patch is OK with those fixes once the <bits/endian.h> changes
> > are reviewed and checked in.  
> 
> Thanks! :)
> 
> I don't add a Reviewed-by tag do I?
> 
> Alistair
> 
> >
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Joseph Myers Sept. 25, 2019, 4:24 p.m. UTC | #4 
On Wed, 25 Sep 2019, Lukasz Majewski wrote:

> > > > +#if __WORDSIZE == 64 \
> > > > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) ||
> > > > \
> > > > +  __TIMESIZE == 32  
>          ^^^^^^^^^^^^^^^^^ - this may be a bit problematic as 32 bit
>          systems (with __TIMESIZE == 32) gain support for 64 bit time
>          (so the Y2038 problem is solved).
> 
> 	 Those systems also would require padding (the code below).
> 
> 	 However, I do guess that allowing such systems to have the
> 	 extra padding after this patch (by removing __TIMESIZE==32
> 	 condition) is not an option for now (and shall be done in the
> 	 same commit which introduces -D_TIME_BITS=64 support to glibc)
> 	 ?

Yes, support for _TIME_BITS=64 should end up as one large commit that adds 
all the header support, all the function exports and symbol versions, all 
the ABI baseline updates and all the documentation.  (It may well be split 
into several pieces to review, but should still end up as one commit to 
avoid intermediate states where some interfaces have _TIME_BITS=64 support 
and some don't, so compiling with _TIME_BITS=64 gives a broken ABI.)
Lukasz Majewski Sept. 25, 2019, 8:05 p.m. UTC | #5 
Hi Joseph,

> On Wed, 25 Sep 2019, Lukasz Majewski wrote:
> 
> > > > > +#if __WORDSIZE == 64 \
> > > > > +  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE ==
> > > > > 64) || \
> > > > > +  __TIMESIZE == 32    
> >          ^^^^^^^^^^^^^^^^^ - this may be a bit problematic as 32 bit
> >          systems (with __TIMESIZE == 32) gain support for 64 bit
> > time (so the Y2038 problem is solved).
> > 
> > 	 Those systems also would require padding (the code below).
> > 
> > 	 However, I do guess that allowing such systems to have the
> > 	 extra padding after this patch (by removing __TIMESIZE==32
> > 	 condition) is not an option for now (and shall be done in
> > the same commit which introduces -D_TIME_BITS=64 support to glibc)
> > 	 ?  
> 
> Yes, support for _TIME_BITS=64 should end up as one large commit that
> adds all the header support, all the function exports and symbol
> versions, all the ABI baseline updates and all the documentation.
> (It may well be split into several pieces to review, but should still
> end up as one commit to avoid intermediate states where some
> interfaces have _TIME_BITS=64 support and some don't, so compiling
> with _TIME_BITS=64 gives a broken ABI.)
> 

Ok. Thanks for clarification.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff mbox

Patch

diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
index 5b77c52b4f0..fd6087955ac 100644
--- a/time/bits/types/struct_timespec.h
+++ b/time/bits/types/struct_timespec.h
@@ -3,13 +3,26 @@ 
 #define _STRUCT_TIMESPEC 1
 
 #include <bits/types.h>
+#include <bits/endian.h>
 
 /* POSIX.1b structure for a time value.  This is like a `struct timeval' but
    has nanoseconds instead of microseconds.  */
 struct timespec
 {
   __time_t tv_sec;		/* Seconds.  */
+#if __WORDSIZE == 64 \
+  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) || \
+  __TIMESIZE == 32
   __syscall_slong_t tv_nsec;	/* Nanoseconds.  */
+#else
+# if __BYTE_ORDER == __BIG_ENDIAN
+  int: 32;           /* Padding */
+  long int tv_nsec;  /* Nanoseconds */
+# else
+  long int tv_nsec;  /* Nanoseconds */
+  int: 32;           /* Padding */
+# endif
+#endif
 };
 
 #endif