[Top][All Lists]

[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:

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.

Attachment: 0001-Merge-mktime-timegm-from-upstream-Gnulib.patch
Description: Text Data

Attachment: 0002-Fix-mktime-localtime-offset-confusion.patch
Description: Text Data

Attachment: 0003-Add-support-for-__time64_t-to-mktime-timegm.patch
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]