Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable

[Joseph Myers <joseph at codesourcery dot com>]

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.

> 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.

>   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_*.

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.

> /* 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.

-- 
Joseph S. Myers
joseph@codesourcery.com