This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/6] y2038: linux: Provide __clock_settime64 implementation


On Mon, 15 Apr 2019, Lukasz Majewski wrote:

> The new 64 bit syscall (clock_settime64) available from Linux
> 5.1+ has been used when applicable on 32 bit systems.
> The execution path on 64 bit systems has not been changed or affected in
> any way.

Is this unchanged specifically because of __TIMESIZE conditionals in the 
code (so that it doesn't matter whether the 64-bit systems define any new 
syscall names)?

Also, I don't see any __ASSUME_* / ENOSYS handling in this patch.  As 
usual, for any new syscall that might be used in code that could also use 
an older syscall:

* If the appropriate __ASSUME_* is defined, the new syscall can simply be 
used unconditionally.

* If the appropriate __ASSUME_* is not defined, there needs to be runtime 
fallback to the old syscall if the new one returns an ENOSYS error (of 
course, that runtime fallback needs to check for the time overflowing the 
32-bit range and give an appropriate error in that case, before calling 
the 32-bit syscall - and it's necessary to figure out what the priority 
should be of errors for invalid nanoseconds values versus overflowing 
seconds).  It is normal and expected for the kernel headers used to build 
glibc to be (much) newer than the oldest kernel version supported by the 
resulting glibc binaries at runtime.

This __ASSUME_* macro definition in kernel-features.h would need a 
multi-paragraph comment discussing exactly what the semantics of such 
__ASSUME_* macros are in the context of the sets of syscall names and 
numbers present on different kinds of Linux kernel architectures.

> +#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;
> +    }

I don't see how an overflow check makes sense in this context (converting 
a 32-bit timespec to a 64-bit one).

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]