Hi Dscho,
Am 26.04.2017 um 00:22 schrieb Johannes Schindelin:
On Tue, 25 Apr 2017, René Scharfe wrote:
Am 24.04.2017 um 15:57 schrieb Johannes Schindelin:
Can we leave time_t alone and just do the part where you replace
unsigned long with timestamp_t defined as uint64_t? That should already
help on Windows, correct? When/if timestamp_t is later changed to a
signed type then we could easily convert the time_t cases to timestamp_t
as well, or the other way around.
This patch series leaves time_t alone already, so your wish has been
fulfilled preemptively.
Sounds good! It does contain conversions from time_t to timestamp_t in
archive-zip.c, though. I'll comment in reply to the relevant patch.
It is arguably a bug to paper over too-large author/committer dates and
to replace them with Jan 1 1970 without even telling the user that we do
that, but this is the behavior that t4212 verifies, so I reinstated that
behavior. The change in behavior was missed because of the missing
unsigned_add_overflows() test.
I can't think of many ways to get future time stamps (broken clock,
broken CMOS battery, bit rot, time travel), so I wouldn't expect a
change towards better error reporting to affect a lot of users. (Not
necessarily as part of this series, of course.)
If you want to suggest that we should stop verifying overflows when a
complex reasoning can prove that the overflow is not happening in a
billion years, I disagree. Not only is it unnecessarily time-consuming to
ask readers to perform the complex reasoning, and not only is there enough
room for bugs to hide in plain sight (because of the complexity), it also
makes the same code harder to reuse in other software where a different
timestamp data type was chosen (or inherited from previous Git versions).
I'd much rather have easy-to-reason code that does not cause head
scratching (like the "why do we ignore a too large timestamp?" triggering
`if (date_overflows(date)) date = 0;`) than pretending to be smart and
clever and make everybody else feel stupid by forcing them through hoops
of thinking bubbles until they also reached the conclusion that this
actually won't happen. Unless there is a bug in the code.
No, I meant that the presence of tests does not necessarily cast an
undesirable behavior into stone, especially if it's hard to trigger
in real life. And that improvements thereof can be done later. In
other words: It's OK to maintain the same behavior in this series,
but cheer up, we may be able to fix the issue eventually.
René