This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable
On Thu, Aug 15, 2019 at 12:46 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 15 Aug 2019, Alistair Francis wrote:
>
> > Is this the type of layout you are looking for? (untested, just a general idea)
>
> I'm not convinced making timespec_get into a wrapper round another
> function is a good thing.
>
> You need to end up, in the __TIMESIZE == 32 case, with two functions that
> have the public timespec_get semantics - not two internal functions and a
> single wrapper - in preparation for when we support _TIME_BITS == 64. So
> I'd expect defining __timespec_get64 as a function with the public
> semantics (not as an internal function that only calls the syscall), and
> conditionally defining timespec_get as a thin wrapper in the case of
> 32-bit time_t, and having a #define of __timespec_get64 to timespec_get in
> an internal header in the case of 64-bit time_t.
Ok, so more like this?
#if __TIMESIZE == 64
# define timespec_get __timespec_get64
#else
# define timespec_get __timespec_get32
#endif
/* Set TS to calendar time based in time base BASE. */
int
__timespec_get64 (struct timespec *ts, int base)
{
switch (base)
{
int res;
INTERNAL_SYSCALL_DECL (err);
case TIME_UTC:
#ifdef __ASSUME_TIME64_SYSCALLS
res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
#else
# ifdef __NR_clock_gettime64
res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
# endif /* __NR_clock_gettime64 */
struct timespec ts32;
if (! in_time_t_range (ts->tv_sec))
{
__set_errno (EOVERFLOW);
return -1;
}
valid_timespec64_to_timespec (ts, &ts32);
res = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);
if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
{
ts->tv_sec = ts32.tv_sec;
ts->tv_nsec = ts32.tv_nsec;
}
#endif
if (INTERNAL_SYSCALL_ERROR_P (res, err))
return 0;
break;
default:
return 0;
}
return base;
}
#if __TIMESIZE != 64
int
__timespec_get32 (struct timespec *ts, int base)
{
struct __timespec64 ts64;
ret = __timespec_get64 (ts64, base);
if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
{
ts->tv_sec = ts64.tv_sec;
ts->tv_nsec = ts64.tv_nsec;
}
return ret;
}
#endif
>
> > int
> > __timespec_get64 (struct __timespec64 *ts, int base)
> > {
> > #ifdef __ASSUME_TIME64_SYSCALLS
> > return INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> > #else
>
> You need
>
> # ifndef __NR_clock_gettime64
> # define __NR_clock_gettime64 __NR_clock_gettime
> # endif
>
> here, because 64-bit platforms define __ASSUME_TIME64_SYSCALLS but with
> unsuffixed syscall names.
The kernel defines 64 suffixed syscalls, is this required because
older kernels don't do this?
>
> > long int ret;
> > # ifdef __NR_clock_gettime64
> > ret = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> >
> > if (ret == 0 || errno != ENOSYS)
> > {
> > return ret;
> > }
>
> You need to check INTERNAL_SYSCALL_ERRNO (...) not errno; errno isn't set
> by INTERNAL_*, only by INLINE_*.
Fixed! Thanks
>
> No braces when only a single statement is inside them.
>
> > ret = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);
> >
> > if ((ret == 0 || errno != ENOSYS) && ts)
>
> No need to consider ENOSYS here. NULL ts isn't valid so no need to check
> for it being non-NULL.
Fixed
>
> > /* Set TS to calendar time based in time base BASE. */
> > int
> > timespec_get (struct timespec *ts, int base)
> > {
> > switch (base)
> > {
> > int res;
> > INTERNAL_SYSCALL_DECL (err);
> > case TIME_UTC:
> > res = __timespec_get (ts, base);
> > if (INTERNAL_SYSCALL_ERROR_P (res, err))
> > return 0;
>
> This wrapper is using an uninitialized variable err.
Good point, fixed.
Alistair
>
> --
> Joseph S. Myers
> joseph@codesourcery.com