[v8,2/3] y2038: Provide conversion helpers for struct __timespec64
Commit Message
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
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.
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
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.)
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
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.
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
@@ -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