Re: [PATCH 1/1] Y2038: add function __difftime64

[Paul Eggert]

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