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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
---
 .../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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux