Message ID | 20190520102723.5380-4-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | y2038: Linux: Provide __clock_settime64 function supporting 64 bit time | expand |
20.05.2019 в 12:27:23 +0200 Lukasz Majewski написал: > /* Set CLOCK to value TP. */ > int > -__clock_settime (clockid_t clock_id, const struct timespec *tp) > +__clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) > { > /* Make sure the time cvalue is OK. */ > if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000) > @@ -32,6 +30,40 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp) > return -1; > } > > +#if __WORDSIZE == 32 > +# ifdef __NR_clock_settime64 > + int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp); > +# ifdef __ASSUME_TIME64_SYSCALLS > + return ret; > +# else > + if (ret == 0 || errno != ENOSYS) > + return ret; > +# endif > +# endif > + /* Fall back to syscall supporting 32bit struct timespec. */ > +# if (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE != 64) > + struct timespec ts32; > + valid_timespec64_to_timespec (tp, &ts32); > + return INLINE_SYSCALL_CALL (clock_settime, clock_id, &ts32); > +# endif > +#endif > return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp); > } This still uses __NR_clock_settime even if __ASSUME_TIME64_SYSCALLS is defined. This won't even compile on newer 32-bit architectures where __NR_clock_settime is not defined. valid_timespec64_to_timespec won't exist in these cases too. And in_time_t_range check is missing for the fallback case. > weak_alias (__clock_settime, clock_settime) > + > +#if __TIMESIZE != 64 > +int > +__clock_settime (clockid_t clock_id, const struct timespec *tp) > +{ > + struct __timespec64 ts64; > + > + if (! in_time_t_range (tp->tv_sec)) > + { > + __set_errno (EOVERFLOW); > + return -1; > + } What is this if (false) { … } statement doing here? > + > + valid_timespec_to_timespec64 (tp, &ts64); > + return __clock_settime64 (clock_id, &ts64); > +} > +#endif
Hi Stepan, First of all - thank you for your reply. > 20.05.2019 в 12:27:23 +0200 Lukasz Majewski написал: > > /* Set CLOCK to value TP. */ > > int > > -__clock_settime (clockid_t clock_id, const struct timespec *tp) > > +__clock_settime64 (clockid_t clock_id, const struct __timespec64 > > *tp) { > > /* Make sure the time cvalue is OK. */ > > if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000) > > @@ -32,6 +30,40 @@ __clock_settime (clockid_t clock_id, const > > struct timespec *tp) return -1; > > } > > > > +#if __WORDSIZE == 32 > > +# ifdef __NR_clock_settime64 > > + int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp); > > +# ifdef __ASSUME_TIME64_SYSCALLS > > + return ret; > > +# else > > + if (ret == 0 || errno != ENOSYS) > > + return ret; > > +# endif > > +# endif > > + /* Fall back to syscall supporting 32bit struct timespec. */ > > +# if (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE != 64) > > + struct timespec ts32; > > + valid_timespec64_to_timespec (tp, &ts32); > > + return INLINE_SYSCALL_CALL (clock_settime, clock_id, &ts32); > > +# endif > > +#endif > > return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp); ^^^^^ - [1] > > } > > This still uses __NR_clock_settime even if __ASSUME_TIME64_SYSCALLS is > defined. This won't even compile on newer 32-bit architectures where > __NR_clock_settime is not defined. valid_timespec64_to_timespec > won't exist in these cases too. Ach... I see your point. The issue would be when we switch to __TIMESIZE == 64 for 32 bit systems. For that reason the "conversion functions" from patch [2] shall be compiled always (no matter if we have __TIMESIZE == 64 or == 32). And some explanation - the if (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE != 64) is to prevent running the fallback on 'x32' - it shall execute on [1] execution path. > > And in_time_t_range check is missing for the fallback case. Yes - I shall move the in_time_t_range() check from __clock_settime to the fallback. Last but not least - the [1] shall be executed only when __WORDSIZE != 64 - the #else is missing > > > weak_alias (__clock_settime, clock_settime) > > + > > +#if __TIMESIZE != 64 > > +int > > +__clock_settime (clockid_t clock_id, const struct timespec *tp) > > +{ > > + struct __timespec64 ts64; > > + > > + if (! in_time_t_range (tp->tv_sec)) > > + { > > + __set_errno (EOVERFLOW); > > + return -1; > > + } > > What is this if (false) { … } statement doing here? Ok, the __clock_settime would receive as an argument the timespec with 32 bit tv_nsec, and tv_sec - hence this check shall be moved from there to the fallback of __clock_settime64. Considering the above changes - the code starts to look too much convoluted. Hence, I would opt for the approach presented in the __ASSUME_TIME64_SYSCALLS patch reply. > > > + > > + valid_timespec_to_timespec64 (tp, &ts64); > > + return __clock_settime64 (clock_id, &ts64); > > +} > > +#endif Note: [2] - [PATCH v4 2/3] y2038: Provide conversion helpers for struct __timespec64 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 --git a/include/time.h b/include/time.h index 6ca4911235..88413d8031 100644 --- a/include/time.h +++ b/include/time.h @@ -127,6 +127,14 @@ extern __time64_t __timegm64 (struct tm *__tp) __THROW; libc_hidden_proto (__timegm64) #endif +#if __TIMESIZE == 64 +# define __clock_settime64 __clock_settime +#else +extern int __clock_settime64 (clockid_t clock_id, + const struct __timespec64 *tp); +libc_hidden_proto (__clock_settime64) +#endif + /* Compute the `struct tm' representation of T, offset OFFSET seconds east of UTC, and store year, yday, mon, mday, wday, hour, min, sec into *TP. diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c index d837e3019c..19559d1e76 100644 --- a/sysdeps/unix/sysv/linux/clock_settime.c +++ b/sysdeps/unix/sysv/linux/clock_settime.c @@ -19,11 +19,9 @@ #include <sysdep.h> #include <time.h> -#include "kernel-posix-cpu-timers.h" - /* Set CLOCK to value TP. */ int -__clock_settime (clockid_t clock_id, const struct timespec *tp) +__clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) { /* Make sure the time cvalue is OK. */ if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000) @@ -32,6 +30,40 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp) return -1; } +#if __WORDSIZE == 32 +# ifdef __NR_clock_settime64 + int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp); +# ifdef __ASSUME_TIME64_SYSCALLS + return ret; +# else + if (ret == 0 || errno != ENOSYS) + return ret; +# endif +# endif + /* Fall back to syscall supporting 32bit struct timespec. */ +# if (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE != 64) + struct timespec ts32; + valid_timespec64_to_timespec (tp, &ts32); + return INLINE_SYSCALL_CALL (clock_settime, clock_id, &ts32); +# endif +#endif return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp); } weak_alias (__clock_settime, clock_settime) + +#if __TIMESIZE != 64 +int +__clock_settime (clockid_t clock_id, const struct timespec *tp) +{ + struct __timespec64 ts64; + + if (! in_time_t_range (tp->tv_sec)) + { + __set_errno (EOVERFLOW); + return -1; + } + + valid_timespec_to_timespec64 (tp, &ts64); + return __clock_settime64 (clock_id, &ts64); +} +#endif