[1/2] y2038: Helper macro to convert struct __timespec64 to struct timespec
Commit Message
This macro allows conversion between Y2038 safe struct __timespec64 and
struct timespec.
The struct __timespec64's tv_nsec field is checked if it is in the correct
range.
Moreover, the tv_sec is asserted if it fits into the time_t range.
When error is detected the errno is set accordingly and the function, which
uses this macro returns -1.
Tested with scripts/build-many-glibcs.py script.
---
include/time.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
Comments
On 10/18/19 7:57 AM, Lukasz Majewski wrote:
> +/* Check if a value lies with the valid nanoseconds range. */
> +#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999)
This should be a static or inline function; there's no need for the
excessive power of a macro here.
> +#define timespec64_to_timespec(ts64) \
> + ({ \
> + if (! IS_VALID_NANOSECONDS (ts64.tv_nsec)) \
> + { \
> + __set_errno (EINVAL); \
> + return -1; \
> + } \
> + if (! in_time_t_range (ts64.tv_sec)) \
> + { \
> + __set_errno (EOVERFLOW); \
> + return -1; \
> + } \
> + valid_timespec64_to_timespec (ts64); })
This macro is too confusing. Instead, if there's a need for this sort of
thing, I suggest a static or inline function that returns true or false
(setting errno); the caller can decide what to do if it returns false.
Hi Paul,
> On 10/18/19 7:57 AM, Lukasz Majewski wrote:
>
> > +/* Check if a value lies with the valid nanoseconds range. */
> > +#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999)
>
> This should be a static or inline function; there's no need for the
> excessive power of a macro here.
Ok. I can add this as an inline static function.
>
> > +#define timespec64_to_timespec(ts64)
> > \
> > + ({
> > \
> > + if (! IS_VALID_NANOSECONDS (ts64.tv_nsec))
> > \
> > + {
> > \
> > + __set_errno (EINVAL);
> > \
> > + return -1;
> > \
> > + }
> > \
> > + if (! in_time_t_range (ts64.tv_sec))
> > \
> > + {
> > \
> > + __set_errno (EOVERFLOW);
> > \
> > + return -1;
> > \
> > + }
> > \
> > + valid_timespec64_to_timespec (ts64); })
>
> This macro is too confusing. Instead, if there's a need for this sort
> of thing, I suggest a static or inline function that returns true or
> false (setting errno);
My first attempt on this conversion helper was based on static inline
function. Unfortunately, such approach has the issue with __set_errno(),
which is not accessible in include/time.h (as it is defined in
include/errno.h).
Moreover, in the glibc the pattern with defining such macros is widely
used - in e.g. math/math-narrow.h or in various sysdep.h headers.
Last but not least - as Joseph pointed out in the other mail - maybe it
would be just enough in this particular case to drop the
IS_VALID_NANOSECONDS() check as this shall be done in the kernel (and
if an error is detected the syscall would fail).
The in_time_t_range() check for clock_getres is also problematic - as
it would be required to have a _really_ bad clock with tv_sec to be
overflowed.
To sum up - for the clock_getres() conversion - I do opt for using
valid_timespec64_to_timespec() (as it is already available in
include/time.h) and drop this patch (but keeping the
IS_VALID_NANOSECONDS() check in the form of static inline).
> the caller can decide what to do if it returns
> false.
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
@@ -236,5 +236,28 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64)
return tv;
}
+
+/* Check if a value lies with the valid nanoseconds range. */
+#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999)
+
+/* Check and convert a struct __timespec64 into a struct timespec.
+ This macro checks if the valid number of nanoseconds has been provided
+ by ts64 and if not the errno is set to EINVAL and -1 is returned.
+ Moreover, the number of seconds is check as well, if it is in the time_t
+ range. If not the errno is set to EOVERFLOW and -1 is returned. */
+#define timespec64_to_timespec(ts64) \
+ ({ \
+ if (! IS_VALID_NANOSECONDS (ts64.tv_nsec)) \
+ { \
+ __set_errno (EINVAL); \
+ return -1; \
+ } \
+ if (! in_time_t_range (ts64.tv_sec)) \
+ { \
+ __set_errno (EOVERFLOW); \
+ return -1; \
+ } \
+ valid_timespec64_to_timespec (ts64); })
+
#endif
#endif