This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH 2/2] y2038: linux: Provide __clock_getres64 implementation
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Lukasz Majewski <lukma at denx dot de>
- Cc: Paul Eggert <eggert at cs dot ucla dot edu>, Alistair Francis <alistair23 at gmail dot com>, Arnd Bergmann <arnd at arndb dot de>, Alistair Francis <alistair dot francis at wdc dot com>, GNU C Library <libc-alpha at sourceware dot org>, Adhemerval Zanella <adhemerval dot zanella at linaro dot org>, Florian Weimer <fweimer at redhat dot com>, Carlos O'Donell <carlos at redhat dot com>, Stepan Golosunov <stepan at golosunov dot pp dot ru>, Florian Weimer <fw at deneb dot enyo dot de>, Zack Weinberg <zackw at panix dot com>
- Date: Fri, 18 Oct 2019 15:38:14 +0000
- Subject: Re: [PATCH 2/2] y2038: linux: Provide __clock_getres64 implementation
- Ironport-sdr: n1RsY68ZjsuGbp/4wGIBPpSHYo7Bf4sEu1vk8AX3kMF1BmqNL8VVc9anfOzoEeAGksKrQtUOHE 8Eh29CWZ8rbD0AIrGIq8fQ1gIMdZbmY81q3T1rYqevaNedlX2VIVEMHoOIifMXdzYFIEzdj54p SCh8mw8W/ktWwFX1A0CczpFNIFN8d8gJzXKOY48r1ZXddBAjdskU3IMrGvocZbjwFmcJytntTT gvB6DBOhAn6mLMBFauJgORSzb+YP8seTDSvMl37o19gqje1eTa8O5ai+S1bx0GMYrJ7eelcSAK iok=
- Ironport-sdr: IAbt7x1bcH2seONV0WD+ijMLaZwWm4BaG9FYLW9vC9zN/P4Z5b9SxbiiFDNgQNLxNQFgdF8thM oTOm6un0su07VnUv02/aUggkqke5Zr20Fv5dnjScIIAjfwmrN0g+bcrNcsovu8t+qlgY7JYUMT YPJBC3xcXF1hWD4rGHXPMNR2aflpoh9NkGMRdyLY+Urru6aTb0cdgE+dXiDva9SRTgPrhNvm+c dL7pbzeZubZaNTX2Li9U6KltPyGGyB30MHyCa0eN7VSXYB4D0QJp/9QX0XI1kzwDJZRYMrHoH3 B/0=
- References: <firstname.lastname@example.org> <email@example.com>
On Fri, 18 Oct 2019, Lukasz Majewski wrote:
> When working on systems still supporting 32 bit time (__TIMESIZE != 64)
> without Y2038 time support the clock_getres returns error when received
> tv_sec excess time_t range (i.e. overflow 32 bit type).
> Moreover, the correctness of tv_nsec is checked.
It's definitely not necessary to check if the kernel returned a valid
tv_nsec value; that can be assumed if the syscall succeeded at all. I'm
doubtful about the check for overflow in this case (a clock whose
resolution exceeds 68 years does not seem a very useful clock), but that
check is at least theoretically relevant.
A stronger justification for the helper macro in patch 1/2 would be if you
have a patch that uses it to check a timespec64 value coming from the
*user* rather than from the kernel (that is, coming from the caller to one
of the __*64 functions that will end up being directly called by code
built with _TIME_BITS=64).
If a function needs to check the nanoseconds value (rather than deferring
to a kernel check of that value), I'd expect it to need to so such a check
regardless of whether it needs to convert 64-bit time to 32-bit time. For
example, that's what __clock_settime64 does - checks nanoseconds before
calling into the kernel at all. So it's not clear to me that there is
actually a use case for a macro that combines a check of the nanoseconds
value with a conversion from 64-bit to 32-bit time.
There *is* definitely a use for a macro such as the IS_VALID_NANOSECONDS
macro in the first patch. Perhaps it would make sense to factor out and
send a patch that just adds that macro and makes existing code with such
checks use it. (See io/ppoll.c, nptl/sem_clockwait.c,
time/clock_nanosleep.c for examples of files with checks that look like
they could use such a macro - not necessarily an exhaustive list. Note
that a few of those files use __builtin_expect, thus suggesting that the
macro definition could use __glibc_likely to reflect that nanoseconds
values almost certainly are valid in real use.)
Joseph S. Myers