[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/1] Y2038: add function __difftime64
From: |
Paul Eggert |
Subject: |
Re: [PATCH 1/1] Y2038: add function __difftime64 |
Date: |
Sat, 1 Sep 2018 00:35:26 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
Albert ARIBAUD wrote:
https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/aaribaud/y2038
Thanks for the heads-up. I would like to look at these patches sequentially.
Let's start with the first three:
Y2038: Add 64-bit time for all architectures
Y2038: make __tz_convert compatible with 64-bit-time
Y2038: make __mktime_internal compatible with 64-bit-time
The first two look OK. The third patch has some problems, some of which I've
mentioned before, some which I haven't:
1. It changes __mktime_internal's offset argument from time_t to __time64_t. But
that offset argument is the difference between localtime and gmtime, which
comfortably fits into long int everywhere; it would even fit into int, for that
matter (the only reason it's a long is that the API was designed for platforms
with 16-bit int). So let's not make that integer wider; let's keep it 'long' (we
could even make it 'int' if we wanted to, since it's private), as that will be
one less thing to hassle with when we change time_t width.
2. It's better to avoid casts when possible, and there are some opportunities
for doing that in the code.
3. The most serious problem is that the patch continues in the tradition of
hoping that integers won't overflow instead of checking for overflow properly.
The upstream code in Gnulib fixed this a couple of years ago, and it's time to
merge that back in now since the old way is likely to be even creakier in the
combined 32-and-64-bit world.
To fix these problems, let's replace this third patch with the attached patchset
0001, 0002, and 0003 instead. I merged the glibc mktime changes upstream into
Gnulib, and 0001 (which may look a little scary due to its size) simply copies
the merged Gnulib code back down to glibc unchanged; this fixes problem (3).
0002 fixes problem (1). 0003 reworks your third patch, except without the casts
so it fixes problem (2).
0001 and 0002 are so straightforward that I've installed their equivalents into
Gnulib on Savannah, so they're properly merged into Gnulib now. (They could be
installed into Glibc now too, as they are independent of your Y2038 changes,
though a review would certainly be nice.) I'd like your opinion on 0003 before
installing its equivalent into Gnulib.
0001-Merge-mktime-timegm-from-upstream-Gnulib.patch
Description: Text Data
0002-Fix-mktime-localtime-offset-confusion.patch
Description: Text Data
0003-Add-support-for-__time64_t-to-mktime-timegm.patch
Description: Text Data
- Re: [PATCH 1/1] Y2038: add function __difftime64,
Paul Eggert <=