From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: Re: [EGIT PATCH] Committer, author and tagger time should not be parsed as 32 bit signed int
Date: Wed, 17 Dec 2008 14:48:44 -0800 [thread overview]
Message-ID: <20081217224844.GK32487@spearce.org> (raw)
In-Reply-To: <1229553172-2038-1-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> If not dates past 2038 will be parsed the wrong way when
> parsed into a RevCommit or RevTag object.
Uhm, sure.
But there's also the commitTime field in RevCommit, its used to
sort commits during walking. In 2038 that will also overflow.
Also, if you search the code for '2038' you'll find a remark
about the year 2038 in DirCacheEntry.smudgeRacilyClean. Last
I looked at the git sources I think this field in the index is
treated as a signed time_t so we can't set the high bits and
extend it out another 60+ years.
Honestly, I'm not sure this patch is worth the code duplication
without fixing our other two known 2038 problem spots... and
I really don't want to make RevCommit.commitTime into a long,
as that will bloat out the object allocations and slow down the
comparsion on 32 bit JVMs. Right now at the end of 2008 the memory
isn't as readily available and there's still a lot of 32 bit JVMs.
Another 10 years we'll probably be looking at 256 bit wide registers
being very common, and 1 PB of core memory, and extending these
fields out to a long will be trivial. And we'll still have 20
years to make the transition.
I'd rather just tag the fields with "2038" so we can search for
them in the future. Like say this:
--8<--
Mark the other two locations that break in the year 2038
When the 32 bit timestamp rolls over in 2038 these spots in JGit
will break, unless we upgrade them to use a 64 bit long before then.
For now its too time and memory intensive to use a full long here,
but in another 10 years we should have enough computing power that
this is a moot point and we can upgrade the code paths marked with
by 'git grep 2038'.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
.../src/org/spearce/jgit/revwalk/RevCommit.java | 2 ++
.../src/org/spearce/jgit/util/RawParseUtils.java | 2 ++
2 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
index 9d30018..bcfd8c4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevCommit.java
@@ -129,6 +129,8 @@ else if (nParents == 1) {
ptr = RawParseUtils.committer(raw, ptr);
if (ptr > 0) {
ptr = RawParseUtils.nextLF(raw, ptr, '>');
+
+ // In 2038 commitTime will overflow unless it is changed to long.
commitTime = RawParseUtils.parseBase10(raw, ptr, null);
}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
index 55a3001..c2d591b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawParseUtils.java
@@ -414,6 +414,8 @@ public static PersonIdent parsePersonIdent(final byte[] raw, final int nameB) {
final String email = decode(cs, raw, emailB, emailE - 1);
final MutableInteger ptrout = new MutableInteger();
+
+ // In 2038 "when" will overflow. Switch to a long before then.
final int when = parseBase10(raw, emailE + 1, ptrout);
final int tz = parseTimeZoneOffset(raw, ptrout.value);
--
1.6.1.rc3.302.gb14d9
--
Shawn.
next prev parent reply other threads:[~2008-12-17 22:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-17 0:07 [EGIT PATCH 0/2] *** SUBJECT HERE *** Robin Rosenberg
2008-12-17 0:07 ` [EGIT PATCH 1/2] Revert "Fix commit id in egit test T0001_ConnectProviderOperationTest" Robin Rosenberg
2008-12-17 0:07 ` [EGIT PATCH 2/2] Fixed an old failed EGit unit test Robin Rosenberg
2008-12-17 16:09 ` [EGIT PATCH 1/2] Revert "Fix commit id in egit test T0001_ConnectProviderOperationTest" Shawn O. Pearce
2008-12-17 22:28 ` Robin Rosenberg
2008-12-17 22:32 ` Shawn O. Pearce
2008-12-17 22:32 ` [EGIT PATCH] Committer, author and tagger time should not be parsed as 32 bit signed int Robin Rosenberg
2008-12-17 22:48 ` Shawn O. Pearce [this message]
2008-12-18 0:09 ` Robin Rosenberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20081217224844.GK32487@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=robin.rosenberg@dewire.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.