git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[JGIT PATCH 1/7] Cleanup RevWalk.parseCommit, parseTree to not throw ClassCastException

From
Shawn O. Pearce <spearce@spearce.org>
Date
Sep 4, 2008, 23:42 UTC
Message-ID
<1220571739-4219-2-git-send-email-spearce@spearce.org>
In-Reply-To
<1220571739-4219-1-git-send-email-spearce@spearce.org>

Throwing ClassCastException here for non-commits is really difficult to work with at the caller level because we may catch the wrong sort of ClassCastException and may mask a bug deep inside of RevWalk's parsing code. It is cleaner to throw IncorrectObjectTypeException and catch that. Besides, the method javadoc says that is what gets thrown if either method is given the wrong type.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../jgit/pgm/opt/AbstractTreeIteratorHandler.java  |    2 --
 .../org/spearce/jgit/pgm/opt/RevCommitHandler.java |    2 --
 .../org/spearce/jgit/pgm/opt/RevTreeHandler.java   |    2 --
 .../src/org/spearce/jgit/revwalk/RevWalk.java      |    7 ++++++-
 .../jgit/transport/BasePackFetchConnection.java    |    2 --
 5 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java
index e439c87..9432d5f 100644
--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java
+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/AbstractTreeIteratorHandler.java
@@ -112,8 +112,6 @@ public int parseArguments(final Parameters params) throws CmdLineException {
 		final CanonicalTreeParser p = new CanonicalTreeParser();
 		try {
 			p.reset(clp.getRepository(), clp.getRevWalk().parseTree(id));
-		} catch (ClassCastException e) {
-			throw new CmdLineException(name + " is not a tree");
 		} catch (MissingObjectException e) {
 			throw new CmdLineException(name + " is not a tree");
 		} catch (IncorrectObjectTypeException e) {
diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java
index 772a8d7..5bfc61e 100644
--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java
+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevCommitHandler.java
@@ -115,8 +115,6 @@ private void addOne(final String name, final boolean interesting)
 		final RevCommit c;
 		try {
 			c = clp.getRevWalk().parseCommit(id);
-		} catch (ClassCastException e) {
-			throw new CmdLineException(name + " is not a commit");
 		} catch (MissingObjectException e) {
 			throw new CmdLineException(name + " is not a commit");
 		} catch (IncorrectObjectTypeException e) {
diff --git a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java
index de64336..0a0c5d2 100644
--- a/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java
+++ b/org.spearce.jgit.pgm/src/org/spearce/jgit/pgm/opt/RevTreeHandler.java
@@ -88,8 +88,6 @@ public int parseArguments(final Parameters params) throws CmdLineException {
 		final RevTree c;
 		try {
 			c = clp.getRevWalk().parseTree(id);
-		} catch (ClassCastException e) {
-			throw new CmdLineException(name + " is not a tree");
 		} catch (MissingObjectException e) {
 			throw new CmdLineException(name + " is not a tree");
 		} catch (IncorrectObjectTypeException e) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
index c90f2a3..243d9b3 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/revwalk/RevWalk.java
@@ -608,6 +608,9 @@ public RevCommit parseCommit(final AnyObjectId id)
 			c = ((RevTag) c).getObject();
 			parse(c);
 		}
+		if (!(c instanceof RevCommit))
+			throw new IncorrectObjectTypeException(id.toObjectId(),
+					Constants.TYPE_COMMIT);
 		return (RevCommit) c;
 	}
 
@@ -639,7 +642,9 @@ public RevTree parseTree(final AnyObjectId id)
 		if (c instanceof RevCommit) {
 			c = ((RevCommit) c).getTree();
 			parse(c);
-		}
+		} else if (!(c instanceof RevTree))
+			throw new IncorrectObjectTypeException(id.toObjectId(),
+					Constants.TYPE_TREE);
 		final RevTree t = (RevTree) c;
 		if (db.openObject(t).getType() != Constants.OBJ_TREE)
 			throw new IncorrectObjectTypeException(t, Constants.TYPE_TREE);
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
index 39fa761..a22b33d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/BasePackFetchConnection.java
@@ -197,8 +197,6 @@ private void markReachable(final int maxTime) throws IOException {
 				reachableCommits.add(o);
 			} catch (IOException readError) {
 				// If we cannot read the value of the ref skip it.
-			} catch (ClassCastException cce) {
-				// Not a commit type.
 			}
 		}
 

-- 
1.6.0.1.319.g9f32b
Previous: Shawn O. PearceNext: Shawn O. Pearce
Message 2 of 8 in “Misc. RevWalk, bundle transport improvements”
  1. 0/7 Misc. RevWalk, bundle transport improvementsShawn O. Pearce, Sep 4, 2008
  2. 1/7 Cleanup RevWalk.parseCommit, parseTree to not throw ClassCastExceptionShawn O. Pearce, Sep 4, 2008
  3. 2/7 Cleanup RevWalk.parseTree semanticsShawn O. Pearce, Sep 4, 2008
  4. 3/7 Fix potential NullPointerException in RevWalk.parseTreeShawn O. Pearce, Sep 4, 2008
  5. 4/7 Add prerequisite verification to the bundle transportShawn O. Pearce, Sep 4, 2008
  6. 5/7 Include URIish in bundle transport within any TransportExceptionsShawn O. Pearce, Sep 4, 2008
  7. 6/7 Refactor TransportBundle to not be dependent on FileInputStreamShawn O. Pearce, Sep 4, 2008
  8. 7/7 Refactor bundle transport to permit streaming from applicationShawn O. Pearce, Sep 4, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.