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: [RFC PATCH 00/52] Make GLIBC Y2038-proof


On Thu, Sep 7, 2017 at 7:21 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> General observations on this patch series:

I concur with all of Joseph's observations and would add:

> * You need to make sure that new ABIs are not added / used on platforms
> where time_t is already 64-bit.

Elaborating on this, when time_t is already 64 bits, defining
_TIME_BITS=64 should have no effect on the generated code, and
defining _TIME_BITS=32 should cause a compile-time error.

* We are not going to land these patches until the kernel side is
finalized.  All of the places where you have "// TODO: implement after
the kernel does" must be replaced with an actual implementation.
Likewise, we need a complete implementation for all supported
architectures before landing.  You're not on the hook to implement
this feature for the Hurd, but you should coordinate with Samuel
Thibault so that at least you do not make glibc-for-Hurd more broken
than it already is.

(But I would encourage you to create a named branch in glibc git for
this project if you haven't already, to make it easier for others to
experiment with it.)

* I understand that you are trying to make the transition as seamless
as possible with _TIME_BITS and so on, but I would seriously question
whether it is appropriate to preserve super-legacy APIs such as
'stime' and 'utime' in the extended mode.  (I'd put the cutoff at
'gettimeofday', which is still heavily used even though
'clock_gettime' officially supersedes it.)

* The POSIX (and ISO C now, argh) requirement for tv_nsec to be 'long'
is, as discussed at great length earlier, incorrect and should be
ignored.  It should instead be a new typedef 'nsec_t' available in
<sys/types.h>, matching the kernel's choice of 32 or 64 bits for this
field (all _t names are reserved for future extension, so the typedef
doesn't need to be _GNU_SOURCE-only). This "willful violation" (as the
HTML5 folks put it) must, of course, be documented.

* We do not use Signed-off-by: and we actively want you to _not_
include it in your patch submissions.  (We have the FSF-mandated
actual copyright assignments instead.)

* The patch series is split too finely.  I recommend breaking it up by
type instead of by function -- time64_t + all the functions that use
only time64_t; struct timeval64 + all the functions that use only
time64_t and/or struct timeval64; and so on.

zw


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