diff mbox

[v8,2/3] y2038: Provide conversion helpers for struct __timespec64

Message ID 20190918211603.8444-3-lukma@denx.de
State New
Headers show

Commit Message

Lukasz Majewski Sept. 18, 2019, 9:16 p.m. UTC
Those functions allow easy conversion between Y2038 safe struct
__timespec64 and other time related data structures (like struct timeval).

* 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.
* include/time.h (IS_VALID_NANOSECONDS): Likewise.
* include/time.h (timespec_to_timespec64): Likewise.
* include/time.h (timespec64_to_timespec): Likewise.
* include/time.h (timespec64_to_timeval): Likewise.

---
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 | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

Comments

Joseph Myers Sept. 19, 2019, 8:17 p.m. UTC | #1
On Wed, 18 Sep 2019, Lukasz Majewski wrote:

> +/* Convert a known valid struct __timespec64 into a struct timespec.  */

It's not just "valid", it's valid *and within range of struct timespec*.  
(That is, I think the comment needs to be expanded.)

> +/* Convert a known valid struct __timespec64 into a struct timeval.  */

Likewise.

> +/* Check and convert a struct timespec into a struct __timespec64.  */
> +static inline bool
> +timespec_to_timespec64 (const struct timespec *ts32,
> +                        struct __timespec64 *ts64)

The comment on the function needs to say what the return value means.

> +/* Check and convert a struct __timespec64 into a struct timespec.  */
> +static inline bool
> +timespec64_to_timespec (const struct __timespec64 *ts64,
> +                        struct timespec *ts32)

Likewise.

> +/* Check and convert a struct __timespec64 into a struct timeval.  */
> +static inline bool
> +timespec64_to_timeval (const struct __timespec64 *ts64,
> +                       struct timeval *tv32)

Likewise.
Lukasz Majewski Sept. 19, 2019, 9:21 p.m. UTC | #2
Hi Joseph,

> On Wed, 18 Sep 2019, Lukasz Majewski wrote:
> 
> > +/* Convert a known valid struct __timespec64 into a struct
> > timespec.  */  
> 
> It's not just "valid", it's valid *and within range of struct
> timespec*. (That is, I think the comment needs to be expanded.)
> 
> > +/* Convert a known valid struct __timespec64 into a struct
> > timeval.  */  
> 
> Likewise.
> 
> > +/* Check and convert a struct timespec into a struct __timespec64.
> >  */ +static inline bool
> > +timespec_to_timespec64 (const struct timespec *ts32,
> > +                        struct __timespec64 *ts64)  
> 
> The comment on the function needs to say what the return value means.
> 
> > +/* Check and convert a struct __timespec64 into a struct timespec.
> >  */ +static inline bool
> > +timespec64_to_timespec (const struct __timespec64 *ts64,
> > +                        struct timespec *ts32)  
> 
> Likewise.
> 
> > +/* Check and convert a struct __timespec64 into a struct timeval.
> > */ +static inline bool
> > +timespec64_to_timeval (const struct __timespec64 *ts64,
> > +                       struct timeval *tv32)  
> 
> Likewise.
> 

I will wait for your review of patch 3/3 of this series ( the one which
converts clock_settime to use __clock_settime64 internally) and send v9
with adjusted comments.

Thanks for the 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
Joseph Myers Sept. 19, 2019, 9:28 p.m. UTC | #3
On Thu, 19 Sep 2019, Lukasz Majewski wrote:

> I will wait for your review of patch 3/3 of this series ( the one which
> converts clock_settime to use __clock_settime64 internally) and send v9
> with adjusted comments.

Please send the new version (after committing patch 1) without waiting for 
such a review.  (At least the proposed commit message for patch 3 should 
be revised to give more details of the configurations in which it was 
tested, even if you don't have any changes for the patch itself at this 
time.)
Lukasz Majewski Sept. 19, 2019, 10:03 p.m. UTC | #4
On Thu, 19 Sep 2019 21:28:50 +0000
Joseph Myers <joseph@codesourcery.com> wrote:

> On Thu, 19 Sep 2019, Lukasz Majewski wrote:
> 
> > I will wait for your review of patch 3/3 of this series ( the one
> > which converts clock_settime to use __clock_settime64 internally)
> > and send v9 with adjusted comments.  
> 
> Please send the new version (after committing patch 1) without
> waiting for such a review.  (At least the proposed commit message for
> patch 3 should be revised to give more details of the configurations
> in which it was tested, even if you don't have any changes for the
> patch itself at this time.)
> 

So if I understood your proposed workflow:

First the patch 1/3 get applied to glibc -master branch. Then on top of
it I shall send the corrected 2/3 (independently from review of 3/3).

Am I correct?


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. 19, 2019, 10:17 p.m. UTC | #5
On Fri, 20 Sep 2019, Lukasz Majewski wrote:

> So if I understood your proposed workflow:
> 
> First the patch 1/3 get applied to glibc -master branch. Then on top of
> it I shall send the corrected 2/3 (independently from review of 3/3).
> 
> Am I correct?

Yes.

In general, if a patch is approved, and it doesn't depend on any other 
unapproved patches, it should be committed (so making future revisions of 
any containing patch series smaller and easier to review) - unless there's 
some special reason to delay the commit until subsequent patches are ready 
as well.
Lukasz Majewski Sept. 19, 2019, 10:22 p.m. UTC | #6
Hi Joseph,

> On Fri, 20 Sep 2019, Lukasz Majewski wrote:
> 
> > So if I understood your proposed workflow:
> > 
> > First the patch 1/3 get applied to glibc -master branch. Then on
> > top of it I shall send the corrected 2/3 (independently from review
> > of 3/3).
> > 
> > Am I correct?  
> 
> Yes.
> 
> In general, if a patch is approved, and it doesn't depend on any
> other unapproved patches, it should be committed (so making future
> revisions of any containing patch series smaller and easier to
> review) - unless there's some special reason to delay the commit
> until subsequent patches are ready as well.
> 

I see. Thanks for explanation.


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/include/time.h b/include/time.h
index 9792948744..64b5468115 100644
--- a/include/time.h
+++ b/include/time.h
@@ -178,5 +178,88 @@  in_time_t_range (__time64_t t)
   return s == t;
 }
 
+/* Convert a known valid struct timeval into a struct __timespec64.  */
+static inline void
+valid_timeval_to_timespec64 (const struct timeval *tv32,
+			     struct __timespec64 *ts64)
+{
+  ts64->tv_sec = tv32->tv_sec;
+  ts64->tv_nsec = tv32->tv_usec * 1000;
+}
+
+/* Convert a known valid struct timespec into a struct __timespec64.  */
+static inline void
+valid_timespec_to_timespec64 (const struct timespec *ts32,
+			      struct __timespec64 *ts64)
+{
+  ts64->tv_sec = ts32->tv_sec;
+  ts64->tv_nsec = ts32->tv_nsec;
+}
+
+/* Convert a known valid struct __timespec64 into a struct timespec.  */
+static inline void
+valid_timespec64_to_timespec (const struct __timespec64 *ts64,
+			      struct timespec *ts32)
+{
+  ts32->tv_sec = (time_t) ts64->tv_sec;
+  ts32->tv_nsec = ts64->tv_nsec;
+}
+
+/* Convert a known valid struct __timespec64 into a struct timeval.  */
+static inline void
+valid_timespec64_to_timeval (const struct __timespec64 *ts64,
+			     struct timeval *tv32)
+{
+  tv32->tv_sec = (time_t) ts64->tv_sec;
+  tv32->tv_usec = ts64->tv_nsec / 1000;
+}
+
+/* Check if a value lies with the valid nanoseconds range.  */
+#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999)
+
+/* Check and convert a struct timespec into a struct __timespec64.  */
+static inline bool
+timespec_to_timespec64 (const struct timespec *ts32,
+                        struct __timespec64 *ts64)
+{
+  /* Check that ts32 holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts32->tv_nsec))
+    return false;
+  /* All ts32 fields can fit in ts64, so copy them.  */
+  valid_timespec_to_timespec64 (ts32, ts64);
+  return true;
+}
+
+/* Check and convert a struct __timespec64 into a struct timespec.  */
+static inline bool
+timespec64_to_timespec (const struct __timespec64 *ts64,
+                        struct timespec *ts32)
+{
+  /* Check that tv_nsec holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts64->tv_nsec))
+    return false;
+  /* Check that tv_sec can fit in a __time_t.  */
+  if (! in_time_t_range (ts64->tv_sec))
+    return false;
+  /* All ts64 fields can fit in ts32, so copy them.  */
+  valid_timespec64_to_timespec (ts64, ts32);
+  return true;
+}
+
+/* Check and convert a struct __timespec64 into a struct timeval.  */
+static inline bool
+timespec64_to_timeval (const struct __timespec64 *ts64,
+                       struct timeval *tv32)
+{
+  /* Check that tv_nsec holds a valid count of nanoseconds.  */
+  if (! IS_VALID_NANOSECONDS (ts64->tv_nsec))
+    return false;
+  /* Check that tv_sec can fit in a __time_t.  */
+  if (! in_time_t_range (ts64->tv_sec))
+    return false;
+  /* All ts64 fields can fit in tv32, so copy them.  */
+  valid_timespec64_to_timeval (ts64, tv32);
+  return true;
+}
 #endif
 #endif