Sun's javac, version 1.5.0_06 and also a Google internal fork of
OpenJDK 1.6.0 both apparently miscompile the for loop which is
looking for a conflicting ref name in the existing set of refs for
this repository.
Debugging this code showed the control flow to return LOCK_FAILURE
when startsWith returned false, which is highly illogical and the
exact opposite of what we have written here.
OpenJDK 1.6.0's javap tool was unable to disassemble the compiled
method. Instead it simply failed to produce anything about
updateImpl. So my remark about the code being compiled wrong
is only a guess based on how I observed the behavior, and not by
actually studying the resulting instructions.
Eclipse 3.4.2's JDT appears to have compiled the updateImpl method
correctly, and produces a working executable. But this is a much
less common compiler to build Java libraries with.
Robin was able to successfully compile the ancestor version of this
code with both javac 1.5.0_19 and also 1.6.0_16, but not everyone
building JGit has these versions installed.
This refactoring to extract the name conflicting test out into its
own method appears to work around the Sun javac bug I observed above,
and the resulting class works correctly with any compiler tested.
The code is also more clear, so its a gain either way.
Signed-off-by: Shawn O. Pearce <sop@xxxxxxxxxx>
---
.../src/org/spearce/jgit/lib/RefUpdate.java | 26 +++++++++++++-------
1 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
index 8dffed2..8226e10 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefUpdate.java
@@ -449,15 +449,8 @@ private Result updateImpl(final RevWalk walk, final Store store)
RevObject newObj;
RevObject oldObj;
- int lastSlash = getName().lastIndexOf('/');
- if (lastSlash > 0)
- if (db.getRepository().getRef(getName().substring(0, lastSlash)) != null)
- return Result.LOCK_FAILURE;
- String rName = getName() + "/";
- for (Ref r : db.getAllRefs().values()) {
- if (r.getName().startsWith(rName))
- return Result.LOCK_FAILURE;
- }
+ if (isNameConflicting())
+ return Result.LOCK_FAILURE;
lock = new LockFile(looseFile);
if (!lock.lock())
return Result.LOCK_FAILURE;
@@ -490,6 +483,21 @@ private Result updateImpl(final RevWalk walk, final Store store)
}
}
+ private boolean isNameConflicting() throws IOException {
+ final String myName = getName();
+ final int lastSlash = myName.lastIndexOf('/');
+ if (lastSlash > 0)
+ if (db.getRepository().getRef(myName.substring(0, lastSlash)) != null)
+ return true;
+
+ final String rName = myName + "/";
+ for (Ref r : db.getAllRefs().values()) {
+ if (r.getName().startsWith(rName))
+ return true;
+ }
+ return false;
+ }
+
private static RevObject safeParse(final RevWalk rw, final AnyObjectId id)
throws IOException {
try {
--
1.6.4.2.395.ge3d52
--
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