[v9] 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 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 | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
Comments
On Thu, 26 Sep 2019, Lukasz Majewski wrote:
> 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.
OK, though we'll need to watch users of these interfaces carefully to see
if timespec64_to_timespec and timespec64_to_timeval are the best
interfaces for the contexts in which they are used. (The issue is that
they return false in two different cases, invalid nanoseconds which would
typically be an EINVAL error and out-of-range seconds which would be
EOVERFLOW. So in any case where the caller needs to distinguish those
conditions to determine what value to use for errno, it would need to
repeat one of the checks from those functions. But maybe the compiler
could optimize the code generated so it doesn't actually end up with a
duplicate of the nanoseconds range test.)
Hi Joseph,
> On Thu, 26 Sep 2019, Lukasz Majewski wrote:
>
> > 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.
>
> OK, though we'll need to watch users of these interfaces carefully to
> see if timespec64_to_timespec and timespec64_to_timeval are the best
> interfaces for the contexts in which they are used. (The issue is
> that they return false in two different cases, invalid nanoseconds
> which would typically be an EINVAL error and out-of-range seconds
> which would be EOVERFLOW. So in any case where the caller needs to
> distinguish those conditions to determine what value to use for
> errno, it would need to repeat one of the checks from those
> functions. But maybe the compiler could optimize the code generated
> so it doesn't actually end up with a duplicate of the nanoseconds
> range test.)
>
So then maybe it would be more natural and practical to just return the
error code when the proper error condition is detected (so the return
would be int instead of bool)?
If everything is OK, return 0.
If we have nanoseconds error, return -EINVAL, and if out-of-range
seconds are detected the helper shall return -EOVERFLOW.
In that way we could avoid the extra error condition check.
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, 27 Sep 2019, Lukasz Majewski wrote:
> > OK, though we'll need to watch users of these interfaces carefully to
> > see if timespec64_to_timespec and timespec64_to_timeval are the best
> > interfaces for the contexts in which they are used. (The issue is
> > that they return false in two different cases, invalid nanoseconds
> > which would typically be an EINVAL error and out-of-range seconds
> > which would be EOVERFLOW. So in any case where the caller needs to
> > distinguish those conditions to determine what value to use for
> > errno, it would need to repeat one of the checks from those
> > functions. But maybe the compiler could optimize the code generated
> > so it doesn't actually end up with a duplicate of the nanoseconds
> > range test.)
> >
>
> So then maybe it would be more natural and practical to just return the
> error code when the proper error condition is detected (so the return
> would be int instead of bool)?
Or remove these functions until we actually have a followup patch using
them (the __clock_settime64 patch doesn't use them), as it will be easier
to judge the best API in the context of patches using it.
On 9/26/19 2:55 PM, Lukasz Majewski wrote:
> +/* 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;
> +}
I suggest using values rather than pointers. That is, this function can accept a
struct timeval and return a struct __timespec64. The *s and the consts just get
in the way for something simple like this, and using values will simplify the
source code.
For the fancier functions that do range checking, you can simply return an
invalid result (with tv_nsec < 0, say). So you don't need a separate boolean
there. Some GNU apps do this sort of thing.
Hi Paul, Joseph,
> On 9/26/19 2:55 PM, Lukasz Majewski wrote:
> > +/* 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;
> > +}
>
> I suggest using values rather than pointers. That is, this function
> can accept a struct timeval and return a struct __timespec64. The *s
> and the consts just get in the way for something simple like this,
> and using values will simplify the source code.
>
Then I will trim the patch to:
1. Only provide:
valid_timeval_to_timespec64 ()
valid_timespec_to_timespec64 ()
valid_timespec64_to_timespec ()
valid_timespec64_to_timeval ()
As those functions are needed for RISC-V 32 glibc port patch series as
well.
2. Above "valid" functions will use values instead of pointers:
static inline struct __timespec64
valid_timespec_to_timespec64 (const struct timespec tp)
{
struct __timespec64 ts64;
ts64.tv_sec = ts32.tv_sec;
ts64.tv_nsec = ts32.tv_nsec;
return ts64;
}
> For the fancier functions that do range checking, you can simply
> return an invalid result (with tv_nsec < 0, say). So you don't need a
> separate boolean there. Some GNU apps do this sort of thing.
In glibc I've already found function which returns struct with all
fields set to 0 (./support/blob_repeat.c - support blob repeat
allocate) and also sets explicitly errno.
In our case we could return -1 in tv_nsec as you suggested.
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,98 @@ 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 valid and within range of struct timespec, 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 valid and within range of struct timeval 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.
+ Returns false if struct timespec's tv_nsec value is not valid.
+ Otherwise returns true. */
+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.
+ Returns false if struct __timespec64's tv_nsec value is not valid or
+ when tv_sec value is outside the range supported by struct timespec's
+ tv_sec. Otherwise it returns true. */
+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.
+ Returns false if struct __timespec64's tv_nsec value is not valid or
+ when tv_sec value is outside the range supported by struct timeval's
+ tv_sec. Otherwise it returns true. */
+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