Patchwork [v10] y2038: Provide conversion helpers for struct __timespec64

login
register
mail settings
Submitter Lukasz Majewski
Date Sept. 30, 2019, 1:31 p.m.
Message ID <20190930133134.14771-1-lukma@denx.de>
Download mbox | patch
Permalink /patch/34728/
State New
Headers show

Comments

Lukasz Majewski - Sept. 30, 2019, 1:31 p.m.
Those functions allow easy conversion between Y2038 safe struct
__timespec64 and other time related data structures (like struct timeval
or struct timespec).

* include/time.h (valid_timeval_to_timespec64): Add.
* include/time.h (valid_timespec_to_timespec64): Likewise.
* include/time.h (valid_timespec64_to_timespec): Likewise.
* include/time.h (valid_timespec64_to_timeval): Likewise.

---
Changes for v10:
- Provide only conversion functions to work on "valid" time structures
  (no need to provide any excessive checks)
- Return values instead of pointers

Changes for v9:
- Update comments to be more concise and describe return values from
  conversion functions (according to Joseph's request).

Changes for v8:
- None

Changes for v7:
- None

Changes for v6:
- Remove the #ifdef guard on __ASSUME_TIME64_SYSCALLS as those functions
  may be needed for fallback execution paths (on e.g. __clock_settime64).

Changes for v5:
- This code is now only compiled in when __ASSUME_TIME64_SYSCALLS is NOT
  defined. Previously it was depending on #if __TIMESIZE != 64.

Changes for v4:
- None

Changes for v3:
- Remove misleading comments regarding clearing tv_pad values during
  conversion (as Linux kernel on its own ignores upper 32 bits of tv_nsec).

Changes for v3:
- Remove timespec64_clear_padding function - as kernel ignores upper 32
  bits of tv_nsec when passed via syscall to the Linux kernel

Changes for v2:
- Add timespec64_clear_padding function
---
 include/time.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)
Joseph Myers - Sept. 30, 2019, 3:30 p.m.
On Mon, 30 Sep 2019, Lukasz Majewski wrote:

> Those functions allow easy conversion between Y2038 safe struct
> __timespec64 and other time related data structures (like struct timeval
> or struct timespec).
> 
> * include/time.h (valid_timeval_to_timespec64): Add.
> * include/time.h (valid_timespec_to_timespec64): Likewise.
> * include/time.h (valid_timespec64_to_timespec): Likewise.
> * include/time.h (valid_timespec64_to_timeval): Likewise.
> 
> ---
> Changes for v10:
> - Provide only conversion functions to work on "valid" time structures
>   (no need to provide any excessive checks)
> - Return values instead of pointers

This patch is OK, please commit.
Joseph Myers - Oct. 1, 2019, 1:25 p.m.
This has broken the build of glibc for i686-gnu as struct timeval isn't 
defined at this point in the build.

In file included from /scratch/jmyers/glibc-bot/install/compilers/i686-gnu/sysroot/include/hurd/hurd_types.h:24,
                 from ../hurd/hurd.h:30,
                 from ../sysdeps/hurd/include/hurd.h:2,
                 from hurdid.c:18:
../include/time.h:183:51: error: parameter 1 ('tv') has incomplete type
 valid_timeval_to_timespec64 (const struct timeval tv)
                              ~~~~~~~~~~~~~~~~~~~~~^~
../include/time.h:221:1: error: return type is an incomplete type
 valid_timespec64_to_timeval (const struct __timespec64 ts64)
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/time.h: In function 'valid_timespec64_to_timeval':
../include/time.h:223:18: error: storage size of 'tv' isn't known
   struct timeval tv;
                  ^~
Lukasz Majewski - Oct. 1, 2019, 2:05 p.m.
Hi Joseph,

> This has broken the build of glibc for i686-gnu as struct timeval
> isn't defined at this point in the build.

I've been using glibc-many.py script and the diff between builds w/
this patch was the same.

I had the UNRESOLVED: for i686-gnu glibc (with and without this patch)
And this seems like the one for hurd...

As there are already patches applied on top of this one - I suppose that
I shall provide incremental fix for it.

> 
> In file included from
> /scratch/jmyers/glibc-bot/install/compilers/i686-gnu/sysroot/include/hurd/hurd_types.h:24,
> from ../hurd/hurd.h:30, from ../sysdeps/hurd/include/hurd.h:2,
>                  from hurdid.c:18:
> ../include/time.h:183:51: error: parameter 1 ('tv') has incomplete
> type valid_timeval_to_timespec64 (const struct timeval tv)
>                               ~~~~~~~~~~~~~~~~~~~~~^~
> ../include/time.h:221:1: error: return type is an incomplete type
>  valid_timespec64_to_timeval (const struct __timespec64 ts64)
>  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../include/time.h: In function 'valid_timespec64_to_timeval':
> ../include/time.h:223:18: error: storage size of 'tv' isn't known
>    struct timeval tv;
>                   ^~
> 

I will submit fix ASAP.


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 - Oct. 1, 2019, 3:25 p.m.
On Tue, 1 Oct 2019, Lukasz Majewski wrote:

> I had the UNRESOLVED: for i686-gnu glibc (with and without this patch)
> And this seems like the one for hurd...

If you had UNRESOLVED rather than FAIL that means the build was broken at 
the time you did the "compilers" build.  You need to do the "compilers" 
build at a time when all glibc configurations are building OK, in order 
for it to be a good basis for testing the "glibcs" build.
Lukasz Majewski - Oct. 1, 2019, 9:54 p.m.
Hi Joseph,

> Hi Joseph,
> 
> > This has broken the build of glibc for i686-gnu as struct timeval
> > isn't defined at this point in the build.  
> 
> I've been using glibc-many.py script and the diff between builds w/
> this patch was the same.
> 
> I had the UNRESOLVED: for i686-gnu glibc (with and without this patch)
> And this seems like the one for hurd...
> 
> As there are already patches applied on top of this one - I suppose
> that I shall provide incremental fix for it.
> 
> > 
> > In file included from
> > /scratch/jmyers/glibc-bot/install/compilers/i686-gnu/sysroot/include/hurd/hurd_types.h:24,
> > from ../hurd/hurd.h:30, from ../sysdeps/hurd/include/hurd.h:2,
> >                  from hurdid.c:18:
> > ../include/time.h:183:51: error: parameter 1 ('tv') has incomplete
> > type valid_timeval_to_timespec64 (const struct timeval tv)
> >                               ~~~~~~~~~~~~~~~~~~~~~^~
> > ../include/time.h:221:1: error: return type is an incomplete type
> >  valid_timespec64_to_timeval (const struct __timespec64 ts64)
> >  ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ../include/time.h: In function 'valid_timespec64_to_timeval':
> > ../include/time.h:223:18: error: storage size of 'tv' isn't known
> >    struct timeval tv;
> >                   ^~
> >   
> 
> I will submit fix ASAP.

It seems to me like it would be best to add the missing
#include <sys/time.h> to HURD's #include <hurd/hurd_types.h> as it also
includes for example struct timespec header. However, is is not so
simple as this file is in hurd compiler includes 
(glibc-many-build/install/compilers/i686-gnu/sysroot/include/hurd/hurd_types.h).

The other option would be to add #include <sys/time.h> just before
struct __timespec64 conversion in ./include/time.h

I'm going to submit such patch for review.

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


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 - Oct. 1, 2019, 9:58 p.m.
On Tue, 1 Oct 2019, Lukasz Majewski wrote:

> It seems to me like it would be best to add the missing
> #include <sys/time.h> to HURD's #include <hurd/hurd_types.h> as it also
> includes for example struct timespec header. However, is is not so
> simple as this file is in hurd compiler includes 
> (glibc-many-build/install/compilers/i686-gnu/sysroot/include/hurd/hurd_types.h).
> 
> The other option would be to add #include <sys/time.h> just before
> struct __timespec64 conversion in ./include/time.h
> 
> I'm going to submit such patch for review.

How about including <bits/types/struct_timeval.h> in include/time.h 
(inside the !_ISOMAC conditional)?  It already includes 
<bits/types/locale_t.h>, for example; since it's using struct timeval, it 
seems natural for it to include the corresponding header (rather than a 
more general sys/time.h include).
Lukasz Majewski - Oct. 1, 2019, 10:15 p.m.
Hi Joseph,

> On Tue, 1 Oct 2019, Lukasz Majewski wrote:
> 
> > It seems to me like it would be best to add the missing
> > #include <sys/time.h> to HURD's #include <hurd/hurd_types.h> as it
> > also includes for example struct timespec header. However, is is
> > not so simple as this file is in hurd compiler includes 
> > (glibc-many-build/install/compilers/i686-gnu/sysroot/include/hurd/hurd_types.h).
> > 
> > The other option would be to add #include <sys/time.h> just before
> > struct __timespec64 conversion in ./include/time.h
> > 
> > I'm going to submit such patch for review.  
> 
> How about including <bits/types/struct_timeval.h> in include/time.h 
> (inside the !_ISOMAC conditional)?  It already includes 
> <bits/types/locale_t.h>, for example; since it's using struct
> timeval, it seems natural for it to include the corresponding header
> (rather than a more general sys/time.h include).
> 

Thanks for the hint. I will prepare and submit (after some testing)
patch using the above approach.


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

Patch

diff --git a/include/time.h b/include/time.h
index 9727786634..9878c2b2ca 100644
--- a/include/time.h
+++ b/include/time.h
@@ -178,5 +178,54 @@  in_time_t_range (__time64_t t)
   return s == t;
 }
 
+/* Convert a known valid struct timeval into a struct __timespec64.  */
+static inline struct __timespec64
+valid_timeval_to_timespec64 (const struct timeval tv)
+{
+  struct __timespec64 ts64;
+
+  ts64.tv_sec = tv.tv_sec;
+  ts64.tv_nsec = tv.tv_usec * 1000;
+
+  return ts64;
+}
+
+/* Convert a known valid struct timespec into a struct __timespec64.  */
+static inline struct __timespec64
+valid_timespec_to_timespec64 (const struct timespec ts)
+{
+  struct __timespec64 ts64;
+
+  ts64.tv_sec = ts.tv_sec;
+  ts64.tv_nsec = ts.tv_nsec;
+
+  return ts64;
+}
+
+/* Convert a valid and within range of struct timespec, struct
+   __timespec64 into a struct timespec.  */
+static inline struct timespec
+valid_timespec64_to_timespec (const struct __timespec64 ts64)
+{
+  struct timespec ts;
+
+  ts.tv_sec = (time_t) ts64.tv_sec;
+  ts.tv_nsec = ts64.tv_nsec;
+
+  return ts;
+}
+
+/* Convert a valid and within range of struct timeval struct
+   __timespec64 into a struct timeval.  */
+static inline struct timeval
+valid_timespec64_to_timeval (const struct __timespec64 ts64)
+{
+  struct timeval tv;
+
+  tv.tv_sec = (time_t) ts64.tv_sec;
+  tv.tv_usec = ts64.tv_nsec / 1000;
+
+  return tv;
+}
 #endif
 #endif