threads / patch / 19181

patch, 7 partsSilence FindBugs "dubious use of GC" in PackFile.mmap

Subject: [JGIT PATCH 2/7] Silence FindBugs "dubious use of GC" in PackFile.mmap

## tl;dr

8 messages between May 4, 2009 and May 6, 2009. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Shawn O. Pearce· May 4, 2009, 23:30 UTC · lore

[JGIT PATCH 1/7] Fix FindBugs exclude filter to work on UNIX platforms

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 org.spearce.jgit/.fbprefs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to org.spearce.jgit/.fbprefs +1 −1

diff --git a/org.spearce.jgit/.fbprefs b/org.spearce.jgit/.fbprefs
index e97968b..d6f94c3 100644
--- a/org.spearce.jgit/.fbprefs
+++ b/org.spearce.jgit/.fbprefs
@@ -119,7 +119,7 @@ detectorWrongMapIterator=WrongMapIterator|true
 detectorXMLFactoryBypass=XMLFactoryBypass|true
 detector_threshold=2
 effort=default
-excludefilter0=findBugs\\FindBugsExcludeFilter.xml
+excludefilter0=findBugs/FindBugsExcludeFilter.xml
 filter_settings=Medium|BAD_PRACTICE,CORRECTNESS,MT_CORRECTNESS,PERFORMANCE,STYLE|false
 filter_settings_neg=MALICIOUS_CODE,NOISE,I18N,SECURITY,EXPERIMENTAL|
 run_at_full_build=true

-- 
1.6.3.rc4.206.g03e16

Shawn O. Pearce· May 4, 2009, 23:30 UTC · re: Shawn O. Pearce · lore

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../findBugs/FindBugsExcludeFilter.xml             |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Show changes to org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml +9 −0

diff --git a/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml b/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml
index 526eb01..2af9348 100644
--- a/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml
+++ b/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml
@@ -6,4 +6,13 @@
        <Method name="refreshPackedRefs" />
        <Bug pattern="DM_STRING_CTOR" />
      </Match>
+
+     <!-- Silence PackFile.mmap calls GC, we need to force it to remove stale
+          memory mapped segments if the JVM heap is out of address space.
+       -->
+     <Match>
+       <Class name="org.spearce.jgit.lib.PackFile" />
+       <Method name="mmap" />
+       <Bug pattern="DM_GC" />
+     </Match>
 </FindBugsFilter>

-- 
1.6.3.rc4.206.g03e16

Shawn O. Pearce· May 4, 2009, 23:30 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 3/7] Fix FindBugs String == is bad form

We only used "a == b" to test for both values == null, as we can't use a.equalsIgnoreCase if a is null. Instead test for that null case, and fall through for everything else.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java +1 −1

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index 87fc254..cb287ee 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -1102,7 +1102,7 @@ boolean match(final String aBase, final String aExtendedBase,
 		}
 
 		private static boolean eq(final String a, final String b) {
-			if (a == b)
+			if (a == null && b == null)
 				return true;
 			if (a == null || b == null)
 				return false;

-- 
1.6.3.rc4.206.g03e16

Shawn O. Pearce· May 4, 2009, 23:30 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 4/7] Fix FindBugs load known null value

We know its null, so use the null constant instead of the null variable here. Either way is pretty clear to the reader, but this form makes FindBugs be quiet.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

Show changes to org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java +2 −3

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index bcbdbb0..d3c43da 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -314,9 +314,8 @@ private static boolean inList(final PackFile[] list, final PackFile pack) {
 
 	private PackFile[] packs() {
 		PackFile[] r = packList.get();
-		if (r == null) {
-			r = scanPacks(r);
-		}
+		if (r == null)
+			r = scanPacks(null);
 		return r;
 	}

-- 
1.6.3.rc4.206.g03e16

Shawn O. Pearce· May 4, 2009, 23:30 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 5/7] FindBugs: Fix use of byte[].toString() in error messages

We meant to use typeString here, not encodedTypeString, as we want a human-readable string form of the object type for display into an exception message. FindBugs caught us using a byte[] here and not a string, resulting in a useless portion of the message text.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/transport/IndexPack.java  |    2 +-
 .../jgit/transport/WalkFetchConnection.java        |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Show changes to 2 files +3 −3

org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java, org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 7881124..b2bcbb7 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -744,7 +744,7 @@ private void verifySafeObject(final AnyObjectId id, final int type,
 				objCheck.check(type, data);
 			} catch (CorruptObjectException e) {
 				throw new IOException("Invalid "
-						+ Constants.encodedTypeString(type) + " " + id.name()
+						+ Constants.typeString(type) + " " + id.name()
 						+ ":" + e.getMessage());
 			}
 		}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
index 06b2c87..4d14305 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/WalkFetchConnection.java
@@ -602,7 +602,7 @@ private void verifyLooseObject(final AnyObjectId id, final byte[] compressed)
 		if (!AnyObjectId.equals(id, idBuffer)) {
 			throw new TransportException("Incorrect hash for " + id.name()
 					+ "; computed " + idBuffer.name() + " as a "
-					+ Constants.encodedTypeString(uol.getType()) + " from "
+					+ Constants.typeString(uol.getType()) + " from "
 					+ compressed.length + " bytes.");
 		}
 		if (objCheck != null) {
@@ -610,7 +610,7 @@ private void verifyLooseObject(final AnyObjectId id, final byte[] compressed)
 				objCheck.check(uol.getType(), uol.getCachedBytes());
 			} catch (CorruptObjectException e) {
 				throw new TransportException("Invalid "
-						+ Constants.encodedTypeString(uol.getType()) + " "
+						+ Constants.typeString(uol.getType()) + " "
 						+ id.name() + ":" + e.getMessage());
 			}
 		}

-- 
1.6.3.rc4.206.g03e16

Shawn O. Pearce· May 4, 2009, 23:30 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 6/7] Make TransportLocal.StreamRewritingThread static

This thread doesn't need access to the outer instance.

Noticed by FindBugs.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/transport/TransportLocal.java |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java +1 −1

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
index dae8951..cffdba1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportLocal.java
@@ -363,7 +363,7 @@ public void close() {
 		}
 	}
 
-	class StreamRewritingThread extends Thread {
+	static class StreamRewritingThread extends Thread {
 		private final InputStream in;
 
 		StreamRewritingThread(final String cmd, final InputStream in) {

-- 
1.6.3.rc4.206.g03e16

Shawn O. Pearce· May 4, 2009, 23:30 UTC · re: Shawn O. Pearce · lore

[JGIT PATCH 7/7] FindBugs: Disable clone() idiom tests

We tend not to implement Cloneable, but instead define our own form of clone() on objects that we want to permit making clones of. This is because Cloneable is a rather poor interface in the JVM with its implicit field copying, but the method name is the right name for us.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 org.spearce.jgit/.fbprefs |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Show changes to org.spearce.jgit/.fbprefs +2 −2

diff --git a/org.spearce.jgit/.fbprefs b/org.spearce.jgit/.fbprefs
index d6f94c3..81a0767 100644
--- a/org.spearce.jgit/.fbprefs
+++ b/org.spearce.jgit/.fbprefs
@@ -1,5 +1,5 @@
 #FindBugs User Preferences
-#Mon May 04 20:17:32 CEST 2009
+#Mon May 04 16:24:13 PDT 2009
 detectorAppendingToAnObjectOutputStream=AppendingToAnObjectOutputStream|true
 detectorBadAppletConstructor=BadAppletConstructor|false
 detectorBadResultSetAccess=BadResultSetAccess|true
@@ -10,7 +10,7 @@ detectorBooleanReturnNull=BooleanReturnNull|true
 detectorCallToUnsupportedMethod=CallToUnsupportedMethod|true
 detectorCheckImmutableAnnotation=CheckImmutableAnnotation|true
 detectorCheckTypeQualifiers=CheckTypeQualifiers|true
-detectorCloneIdiom=CloneIdiom|true
+detectorCloneIdiom=CloneIdiom|false
 detectorComparatorIdiom=ComparatorIdiom|true
 detectorConfusedInheritance=ConfusedInheritance|true
 detectorConfusionBetweenInheritedAndOuterMethod=ConfusionBetweenInheritedAndOuterMethod|true

-- 
1.6.3.rc4.206.g03e16

Sohn, Matthias· May 6, 2009, 11:18 UTC · re: Shawn O. Pearce · lore

RE: [JGIT PATCH 1/7] Fix FindBugs exclude filter to work on UNIX platforms

Shawn O. Pearce [mailto:spearce@spearce.org] wrote:

> Subject: [JGIT PATCH 1/7] Fix FindBugs exclude filter to work on UNIX
> platforms

Thanks, I missed this. I reported this problem to FindBugs : https://sourceforge.net/tracker/index.php?func=detail&aid=2787728&group_id=96405&atid=614693

-- Matthias