git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 1/2] Improve end-of-file detection in DirCache
Date: Thu,  9 Apr 2009 08:28:18 -0700	[thread overview]
Message-ID: <1239290899-24589-1-git-send-email-spearce@spearce.org> (raw)
In-Reply-To: <1239205852-28138-1-git-send-email-robin.rosenberg@dewire.com>

When reading from a BufferInputStream attached to a FileInputStream
the available() method seems to return the number of unread bytes
in the buffer plus the unread number of bytes in the file.  There is
no guarantee for this behavior, so we can't rely on it.

Instead we read the expected 20 byte SHA1 trailer into a temporary
buffer, and then poll for a remaining byte.  If another byte is
still present in the input stream it indicates that there is more
than 20 bytes worth of data remaining, which means an extension
must be available at the current position.  If there is in fact
an extension we put back the 21 bytes we read and proceed to do
an 8 byte read for the extension header.

This relies on BufferInputStream's ability to buffer at least 21
bytes of data, and put them all back in the event that we found
an extension.  This is a much more common usage of the stream API
and is something we can rely on working correctly all of the time,
on any standard library implementation.

Found-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 I'd rather do this instead.  Its accurate 100% of the time and isn't
 a hack.  So I'm dropping the patch I'm replying to and asking you
 to review and apply this 2 patch series (or at least this first
 one which should fix the problem).
 
 .../src/org/spearce/jgit/dircache/DirCache.java    |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java
index 8eb4022..5df0f59 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/dircache/DirCache.java
@@ -45,7 +45,6 @@
 import java.io.IOException;
 import java.io.OutputStream;
 import java.nio.ByteBuffer;
-import java.nio.channels.FileChannel;
 import java.security.DigestOutputStream;
 import java.security.MessageDigest;
 import java.util.Comparator;
@@ -344,13 +343,11 @@ public void clear() {
 
 	private void readFrom(final FileInputStream inStream) throws IOException,
 			CorruptObjectException {
-		final FileChannel fd = inStream.getChannel();
-		final long sizeOnDisk = fd.size();
 		final BufferedInputStream in = new BufferedInputStream(inStream);
 
 		// Read the index header and verify we understand it.
 		//
-		final byte[] hdr = new byte[12];
+		final byte[] hdr = new byte[20];
 		NB.readFully(in, hdr, 0, 12);
 		if (!is_DIRC(hdr))
 			throw new CorruptObjectException("Not a DIRC file.");
@@ -369,13 +366,22 @@ private void readFrom(final FileInputStream inStream) throws IOException,
 			sortedEntries[i] = new DirCacheEntry(infos, i * INFO_LEN, in);
 		lastModified = liveFile.lastModified();
 
-		// After the file entries are index extensions.
+		// After the file entries are index extensions, and then a footer.
 		//
-		while (fd.position() - in.available() < sizeOnDisk - 20) {
-			NB.readFully(in, hdr, 0, 8);
+		for (;;) {
+			in.mark(21);
+			NB.readFully(in, hdr, 0, 20);
+			if (in.read() < 0) {
+				// No extensions present; the file ended where we expected.
+				//
+				break;
+			}
+			in.reset();
+
 			switch (NB.decodeInt32(hdr, 0)) {
 			case EXT_TREE: {
 				final byte[] raw = new byte[NB.decodeInt32(hdr, 4)];
+				NB.skipFully(in, 8);
 				NB.readFully(in, raw, 0, raw.length);
 				tree = new DirCacheTree(raw, new MutableInteger(), null);
 				break;
-- 
1.6.2.2.600.g24c24

  reply	other threads:[~2009-04-09 15:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-08 15:50 [EGIT PATCH] Improve end-of-file detection in DirCache Robin Rosenberg
2009-04-09 15:28 ` Shawn O. Pearce [this message]
2009-04-09 15:28   ` [JGIT PATCH 2/2] Validate the DIRC footer during read Shawn O. Pearce
2009-04-10  2:35   ` [JGIT PATCH 1/2] Improve end-of-file detection in DirCache Robin Rosenberg
2009-04-13 11:53   ` Robin Rosenberg
2009-04-13 15:42     ` Shawn O. Pearce

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=1239290899-24589-1-git-send-email-spearce@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).