Re: [PATCH v8] y2038: Introduce __ASSUME_TIME64_SYSCALLS define

[Alistair Francis <alistair23 at gmail dot com>]

On Fri, Aug 16, 2019 at 8:49 AM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 15 Aug 2019, Alistair Francis wrote:
>
> > /* Support for the 64-bit time Linux kernel syscalls.
> >
> >    This flag indicates support for Linux kernel syscalls, which are able
> >    to handle the 64 bit time ABI. It is defined for all 64-bit architectures as
>
> I think "syscalls, which are able to handle the 64 bit time ABI" is
> awkward.  "syscalls using 64-bit time" or similar would seem better.
> (Throughout, use "64-bit" with a hyphen as an adjective.)
>
> >    they have always supported 64 bit time support. It is also defined for all
> >    32-bit architectures when using Linux kernel version 5.1 or newer.
>
> "always supported 64 bit time support" should lose the last "support" (and
> add the hyphen in 64-bit).
>
> >    When __ASSUME_TIME64_SYSCALLS is defined glibc should call the *64/time64
> >    suffixed syscalls. These should be #defined to the the unsuffixed versions
> >    when required (such as when running on 64-bit systems).
>
> I don't think the interface definition involves "should call".  It's more
> like "can call, without needing to check at runtime for ENOSYS errors".
>
> >    On systems with __WORDSIZE == 64 the __NR_clock_settime syscall is used
> >    to achieve this goal. Contrary, systems with __WORDSIZE == 32 do use
> >    new __NR_clock_settime64 syscall available from Linux version 5.1.
>
> Describing syscalls as "new" is a bad idea.  (You can also remove
> "Contrary" and "do".)
>
> >    The __ASSUME_TIME64_SYSCALLS is defined for:
>
> The __ASSUME_TIME64_SYSCALLS *macro*.
>
> >    1. Systems with intrinsic 64 bit time support (__WORDSIZE == 64).
>
> As I said in <https://sourceware.org/ml/libc-alpha/2019-05/msg00688.html>,
> don't use this "intrinsic" description.
>
> >    3. Systems with __WORDSIZE==32, which gain 64 bit time support
> >       with new set of syscalls added to Linux kernel 5.1.
>
> And avoid "new".
>
> >    4. All new 32-bit architectures that only support 64-bit time, such as RV32.
>
> Again, avoid "new"; describe the actual properties involved so the comment
> doesn't depend on people reading it from a 2019 perspective in which
> certain things count as "new".  I think you mean 32-bit architectures for
> which support was added in Linux 5.1 or later, or for which the
> kernel/userspace ABI changed incompatibly in Linux 5.1 or later so that
> there are no syscalls using 32-bit time.

Thanks for the review.

I just talked to Lukasz and he is happy with me sending out the next
version of this patch so I'll send that now.

Alistair

>
> --
> Joseph S. Myers
> joseph@codesourcery.com