[JGIT PATCH 00/12] Cleanup Config class

[JGIT PATCH 00/12] Cleanup Config class

[Shawn O. Pearce]

Misc. cleanups of the Config class (aka the base class for
RepositoryConfig) to make the code easier to follow.  I came
about this series because I started to do improvements to the
Config class to better support submodule introspection from a bare
repository... and I realized this code was a mess.

This series (or at least the first patch of it) applies on top of
my prior 11 patch series for alternate object database improvements.

Shawn O. Pearce (12):
  Use NB.readFully(File) to slurp complete file contents
  Correct name of fileRead member of Config class
  Add setLong to Config
  Fix Config setInt(..., 0) to store "0" not "0 g"
  Rename Config.unsetString to just unset()
  Remove pointless null assignments in Config
  Clarify section and subsection values in Config code
  Don't subclass PrintWriter when writing the Config
  Use a Java 5 style iteration over the Config entries list
  Match config subsection names using case sensitive search
  Cleanup Config's MAGIC_EMPTY_VALUE to be more safe
  Remove unreferenced REMOTE_SECTION from RepositoryConfig

 .../src/org/spearce/jgit/lib/Config.java           |  196 +++++++++++---------
 .../src/org/spearce/jgit/lib/FileBasedConfig.java  |    7 +-
 .../src/org/spearce/jgit/lib/RefDatabase.java      |   15 +-
 .../src/org/spearce/jgit/lib/ReflogReader.java     |   32 +---
 .../src/org/spearce/jgit/lib/RepositoryCache.java  |   20 +-
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |    4 -
 .../org/spearce/jgit/lib/UnpackedObjectLoader.java |   15 +--
 .../org/spearce/jgit/transport/RemoteConfig.java   |    2 +-
 org.spearce.jgit/src/org/spearce/jgit/util/NB.java |   52 +++++
 9 files changed, 195 insertions(+), 148 deletions(-)

[JGIT PATCH 01/12] Use NB.readFully(File) to slurp complete file contents

[Shawn O. Pearce]

When we want the entire file contents and its likely a very small
file (e.g. a ref, configuration file) NB.readFully(File) is an
easier method to get the content then using FileInputStream from
application code.

When looking at a file that we believe should contain only a SHA-1
or be a symbolic ref to another file within this repository,
we use a maximum read cap of 4096 bytes for the slurp, so that
we don't accidentally allocate a very large buffer and blow the
JVM heap up.  This reasonableness cap is particularly important
for the RepositoryCache.FileKey.isGitRepository() test logic, in
case it comes across a file called HEAD that isn't really part of
a Git repository.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Due to the patch to RepositoryCache here, this applies on top of
 my other 11 patch series that introduced the class.

 .../src/org/spearce/jgit/lib/RefDatabase.java      |   15 +++--
 .../src/org/spearce/jgit/lib/ReflogReader.java     |   32 ++++--------
 .../src/org/spearce/jgit/lib/RepositoryCache.java  |   20 +++----
 .../org/spearce/jgit/lib/UnpackedObjectLoader.java |   15 +-----
 org.spearce.jgit/src/org/spearce/jgit/util/NB.java |   52 ++++++++++++++++++++
 5 files changed, 81 insertions(+), 53 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
index 383877f..f7751c4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RefDatabase.java
@@ -52,6 +52,8 @@
 import org.spearce.jgit.errors.ObjectWritingException;
 import org.spearce.jgit.lib.Ref.Storage;
 import org.spearce.jgit.util.FS;
+import org.spearce.jgit.util.NB;
+import org.spearce.jgit.util.RawParseUtils;
 
 class RefDatabase {
 	private static final String REFS_SLASH = "refs/";
@@ -494,12 +496,13 @@ protected void writeFile(String name, byte[] content) throws IOException {
 
 	private static String readLine(final File file)
 			throws FileNotFoundException, IOException {
-		final BufferedReader br = openReader(file);
-		try {
-			return br.readLine();
-		} finally {
-			br.close();
-		}
+		final byte[] buf = NB.readFully(file, 4096);
+		int n = buf.length;
+		if (n == 0)
+			return null;
+		if (buf[n - 1] == '\n')
+			n--;
+		return RawParseUtils.decode(buf, 0, n);
 	}
 
 	private static BufferedReader openReader(final File fileLocation)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java
index e86a723..5f2d28c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ReflogReader.java
@@ -37,7 +37,6 @@
 package org.spearce.jgit.lib;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -157,31 +156,20 @@ public Entry getLastEntry() throws IOException {
 	 * @throws IOException
 	 */
 	public List<Entry> getReverseEntries(int max) throws IOException {
-		FileInputStream fileInputStream;
+		final byte[] log;
 		try {
-			fileInputStream = new FileInputStream(logName);
+			log = NB.readFully(logName);
 		} catch (FileNotFoundException e) {
 			return Collections.emptyList();
 		}
-		try {
-			long logSize = fileInputStream.getChannel().size();
-			if (logSize > Integer.MAX_VALUE) {
-				// implementation limit, will suck with smaller files too
-				throw new IOException("Cannot handle reflog larger than "
-						+ Integer.MAX_VALUE + " bytes");
-			}
-			byte[] log = new byte[(int) logSize];
-			NB.readFully(fileInputStream, log, 0, log.length);
-			int rs = RawParseUtils.prevLF(log, log.length);
-			List<Entry> ret = new ArrayList<Entry>();
-			while (rs >= 0 && max-- > 0) {
-				rs = RawParseUtils.prevLF(log, rs);
-				Entry entry = new Entry(log, rs < 0 ? 0 : rs + 2);
-				ret.add(entry);
-			}
-			return ret;
-		} finally {
-			fileInputStream.close();
+
+		int rs = RawParseUtils.prevLF(log, log.length);
+		List<Entry> ret = new ArrayList<Entry>();
+		while (rs >= 0 && max-- > 0) {
+			rs = RawParseUtils.prevLF(log, rs);
+			Entry entry = new Entry(log, rs < 0 ? 0 : rs + 2);
+			ret.add(entry);
 		}
+		return ret;
 	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryCache.java
index 50b4330..5f90b55 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryCache.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryCache.java
@@ -37,11 +37,8 @@
 
 package org.spearce.jgit.lib;
 
-import java.io.BufferedReader;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
 import java.util.Iterator;
@@ -50,6 +47,8 @@
 
 import org.spearce.jgit.errors.RepositoryNotFoundException;
 import org.spearce.jgit.util.FS;
+import org.spearce.jgit.util.NB;
+import org.spearce.jgit.util.RawParseUtils;
 
 /** Cache of active {@link Repository} instances. */
 public class RepositoryCache {
@@ -341,14 +340,13 @@ private static boolean isValidHead(final File head) {
 
 		private static String readFirstLine(final File head) {
 			try {
-				final BufferedReader br = new BufferedReader(
-						new InputStreamReader(new FileInputStream(head),
-								Constants.CHARSET));
-				try {
-					return br.readLine();
-				} finally {
-					br.close();
-				}
+				final byte[] buf = NB.readFully(head, 4096);
+				int n = buf.length;
+				if (n == 0)
+					return null;
+				if (buf[n - 1] == '\n')
+					n--;
+				return RawParseUtils.decode(buf, 0, n);
 			} catch (IOException e) {
 				return null;
 			}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java
index e2de41c..b5abeb9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UnpackedObjectLoader.java
@@ -39,7 +39,6 @@
 package org.spearce.jgit.lib;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.util.zip.DataFormatException;
@@ -74,19 +73,7 @@
 	 */
 	public UnpackedObjectLoader(final File path, final AnyObjectId id)
 			throws IOException {
-		this(readCompressed(path), id);
-	}
-
-	private static byte[] readCompressed(final File path)
-			throws FileNotFoundException, IOException {
-		final FileInputStream in = new FileInputStream(path);
-		try {
-			final byte[] compressed = new byte[(int) in.getChannel().size()];
-			NB.readFully(in, compressed, 0, compressed.length);
-			return compressed;
-		} finally {
-			in.close();
-		}
+		this(NB.readFully(path), id);
 	}
 
 	/**
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/NB.java b/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
index 032997f..17b3398 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/NB.java
@@ -38,6 +38,9 @@
 package org.spearce.jgit.util;
 
 import java.io.EOFException;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteBuffer;
@@ -46,6 +49,55 @@
 /** Conversion utilities for network byte order handling. */
 public final class NB {
 	/**
+	 * Read an entire local file into memory as a byte array.
+	 *
+	 * @param path
+	 *            location of the file to read.
+	 * @return complete contents of the requested local file.
+	 * @throws FileNotFoundException
+	 *             the file does not exist.
+	 * @throws IOException
+	 *             the file exists, but its contents cannot be read.
+	 */
+	public static final byte[] readFully(final File path)
+			throws FileNotFoundException, IOException {
+		return readFully(path, Integer.MAX_VALUE);
+	}
+
+	/**
+	 * Read an entire local file into memory as a byte array.
+	 *
+	 * @param path
+	 *            location of the file to read.
+	 * @param max
+	 *            maximum number of bytes to read, if the file is larger than
+	 *            this limit an IOException is thrown.
+	 * @return complete contents of the requested local file.
+	 * @throws FileNotFoundException
+	 *             the file does not exist.
+	 * @throws IOException
+	 *             the file exists, but its contents cannot be read.
+	 */
+	public static final byte[] readFully(final File path, final int max)
+			throws FileNotFoundException, IOException {
+		final FileInputStream in = new FileInputStream(path);
+		try {
+			final long sz = in.getChannel().size();
+			if (sz > max)
+				throw new IOException("File is too large: " + path);
+			final byte[] buf = new byte[(int) sz];
+			readFully(in, buf, 0, buf.length);
+			return buf;
+		} finally {
+			try {
+				in.close();
+			} catch (IOException ignored) {
+				// ignore any close errors, this was a read only stream
+			}
+		}
+	}
+
+	/**
 	 * Read the entire byte array into memory, or throw an exception.
 	 * 
 	 * @param fd
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 02/12] Correct name of fileRead member of Config class

[Shawn O. Pearce]

The setter method is called setFileRead() so the member should
have been called fileRead, not readFile.

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index c2d5c6e..a31796c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -60,8 +60,7 @@
  * file.
  */
 public abstract class Config {
-
-	private boolean readFile;
+	private boolean fileRead;
 
 	private List<Entry> entries;
 
@@ -96,7 +95,7 @@ protected Config(Config base) {
 	 *            true if file does not need loading
 	 */
 	protected void setFileRead(boolean ok) {
-		readFile = ok;
+		fileRead = ok;
 	}
 
 	/**
@@ -404,7 +403,7 @@ private String getRawString(final String section, final String subsection,
 	}
 
 	private void ensureLoaded() {
-		if (!readFile) {
+		if (!fileRead) {
 			try {
 				load();
 			} catch (FileNotFoundException err) {
@@ -703,7 +702,7 @@ protected void printConfig(final PrintWriter r) {
 	 */
 	public void load() throws IOException {
 		clear();
-		readFile = true;
+		fileRead = true;
 		final BufferedReader r = new BufferedReader(new InputStreamReader(
 				openInputStream(), Constants.CHARSET));
 		try {
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 03/12] Add setLong to Config

[Shawn O. Pearce]

We already support getLong(), but didn't support setting one.  The
setInt() method can be trivially expressed in terms of setLong().

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index a31796c..6fbdab9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -449,6 +449,29 @@ private Object getRawEntry(final String section, final String subsection,
 	 */
 	public void setInt(final String section, final String subsection,
 			final String name, final int value) {
+		setLong(section, subsection, name, value);
+	}
+
+	/**
+	 * Add or modify a configuration value. The parameters will result in a
+	 * configuration entry like this.
+	 *
+	 * <pre>
+	 * [section &quot;subsection&quot;]
+	 *         name = value
+	 * </pre>
+	 *
+	 * @param section
+	 *            section name, e.g "branch"
+	 * @param subsection
+	 *            optional subsection value, e.g. a branch name
+	 * @param name
+	 *            parameter name, e.g. "filemode"
+	 * @param value
+	 *            parameter value
+	 */
+	public void setLong(final String section, final String subsection,
+			final String name, final long value) {
 		final String s;
 
 		if ((value % (1024 * 1024 * 1024)) == 0)
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 04/12] Fix Config setInt(..., 0) to store "0" not "0 g"

[Shawn O. Pearce]

If the value is less than the unit's minimum value, we don't want to
store the value with the unit suffix, but instead must try the next
lower unit's value.

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index 6fbdab9..20f42c4 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -60,6 +60,10 @@
  * file.
  */
 public abstract class Config {
+	private static final long KiB = 1024;
+	private static final long MiB = 1024 * KiB;
+	private static final long GiB = 1024 * MiB;
+
 	private boolean fileRead;
 
 	private List<Entry> entries;
@@ -230,13 +234,13 @@ public long getLong(final String section, String subsection,
 		long mul = 1;
 		switch (Character.toLowerCase(n.charAt(n.length() - 1))) {
 		case 'g':
-			mul = 1024 * 1024 * 1024;
+			mul = GiB;
 			break;
 		case 'm':
-			mul = 1024 * 1024;
+			mul = MiB;
 			break;
 		case 'k':
-			mul = 1024;
+			mul = KiB;
 			break;
 		}
 		if (mul > 1)
@@ -473,13 +477,13 @@ public void setInt(final String section, final String subsection,
 	public void setLong(final String section, final String subsection,
 			final String name, final long value) {
 		final String s;
-
-		if ((value % (1024 * 1024 * 1024)) == 0)
-			s = String.valueOf(value / (1024 * 1024 * 1024)) + " g";
-		else if ((value % (1024 * 1024)) == 0)
-			s = String.valueOf(value / (1024 * 1024)) + " m";
-		else if ((value % 1024) == 0)
-			s = String.valueOf(value / 1024) + " k";
+		
+		if (value >= GiB && (value % GiB) == 0)
+			s = String.valueOf(value / GiB) + " g";
+		else if (value >= MiB && (value % MiB) == 0)
+			s = String.valueOf(value / MiB) + " m";
+		else if (value >= KiB && (value % KiB) == 0)
+			s = String.valueOf(value / KiB) + " k";
 		else
 			s = String.valueOf(value);
 
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 05/12] Rename Config.unsetString to just unset()

[Shawn O. Pearce]

Removing a configuration option means deleting it from the file,
implying a value type during the deletion makes no sense.

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index 20f42c4..4a60199 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -547,7 +547,7 @@ public void setString(final String section, final String subsection,
 	 * @param name
 	 *            parameter name, e.g. "filemode"
 	 */
-	public void unsetString(final String section, final String subsection,
+	public void unset(final String section, final String subsection,
 			final String name) {
 		setStringList(section, subsection, name, Collections
 				.<String> emptyList());
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java
index fac04f6..93a5873 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/RemoteConfig.java
@@ -247,7 +247,7 @@ private void set(final RepositoryConfig rc, final String key,
 	}
 
 	private void unset(final RepositoryConfig rc, final String key) {
-		rc.unsetString(SECTION, getName(), key);
+		rc.unset(SECTION, getName(), key);
 	}
 
 	/**
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 06/12] Remove pointless null assignments in Config

[Shawn O. Pearce]

The new instance already initialized these fields to null.  We
don't set them elsewhere unless we want them to be non-null,
so we shouldn't set them here either.

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index 4a60199..4940cc2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -639,8 +639,6 @@ else if (values.size() == 1) {
 				// so we must create a new section header at the end.
 				//
 				final Entry e = new Entry();
-				e.prefix = null;
-				e.suffix = null;
 				e.base = section;
 				e.extendedBase = subsection;
 				entries.add(e);
@@ -648,8 +646,6 @@ else if (values.size() == 1) {
 			}
 			while (valueIndex < values.size()) {
 				final Entry e = new Entry();
-				e.prefix = null;
-				e.suffix = null;
 				e.base = section;
 				e.extendedBase = subsection;
 				e.name = name;
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 07/12] Clarify section and subsection values in Config code

[Shawn O. Pearce]

When I first wrote this code I wasn't clear in my intent; the section
name was sometimes called "base" and the subsection name was somtimes
called "extendedBase".  This is nuts, everywhere else in our code we
use "section" and "subsection".  Renaming the fields makes the code
much easier to follow.

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index 4940cc2..a2934a2 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -374,8 +374,8 @@ public String getString(final String section, String subsection,
 		ensureLoaded();
 
 		for (final Entry e : entries) {
-			if (section.equalsIgnoreCase(e.base) && e.extendedBase != null)
-				result.add(e.extendedBase);
+			if (section.equalsIgnoreCase(e.section) && e.subsection != null)
+				result.add(e.subsection);
 		}
 		if (baseConfig != null)
 			result.addAll(baseConfig.getSubsections(section));
@@ -582,8 +582,8 @@ public void setStringList(final String section, final String subsection,
 			byName.remove(key);
 		else if (values.size() == 1) {
 			final Entry e = new Entry();
-			e.base = section;
-			e.extendedBase = subsection;
+			e.section = section;
+			e.subsection = subsection;
 			e.name = name;
 			e.value = values.get(0);
 			byName.put(key, e);
@@ -591,8 +591,8 @@ else if (values.size() == 1) {
 			final ArrayList<Entry> eList = new ArrayList<Entry>(values.size());
 			for (final String v : values) {
 				final Entry e = new Entry();
-				e.base = section;
-				e.extendedBase = subsection;
+				e.section = section;
+				e.subsection = subsection;
 				e.name = name;
 				e.value = v;
 				eList.add(e);
@@ -639,15 +639,15 @@ else if (values.size() == 1) {
 				// so we must create a new section header at the end.
 				//
 				final Entry e = new Entry();
-				e.base = section;
-				e.extendedBase = subsection;
+				e.section = section;
+				e.subsection = subsection;
 				entries.add(e);
 				insertPosition = entries.size();
 			}
 			while (valueIndex < values.size()) {
 				final Entry e = new Entry();
-				e.base = section;
-				e.extendedBase = subsection;
+				e.section = section;
+				e.subsection = subsection;
 				e.name = name;
 				e.value = values.get(valueIndex++);
 				entries.add(insertPosition++, e);
@@ -686,17 +686,17 @@ protected void printConfig(final PrintWriter r) {
 			if (e.prefix != null) {
 				r.print(e.prefix);
 			}
-			if (e.base != null && e.name == null) {
+			if (e.section != null && e.name == null) {
 				r.print('[');
-				r.print(e.base);
-				if (e.extendedBase != null) {
+				r.print(e.section);
+				if (e.subsection != null) {
 					r.print(' ');
 					r.print('"');
-					r.print(escapeValue(e.extendedBase));
+					r.print(escapeValue(e.subsection));
 					r.print('"');
 				}
 				r.print(']');
-			} else if (e.base != null && e.name != null) {
+			} else if (e.section != null && e.name != null) {
 				if (e.prefix == null || "".equals(e.prefix)) {
 					r.print('\t');
 				}
@@ -740,7 +740,7 @@ public void load() throws IOException {
 				} else if ('\n' == in) {
 					// End of this entry.
 					add(e);
-					if (e.base != null) {
+					if (e.section != null) {
 						last = e;
 					}
 					e = new Entry();
@@ -750,18 +750,18 @@ public void load() throws IOException {
 				} else if (';' == in || '#' == in) {
 					// The rest of this line is a comment; put into suffix.
 					e.suffix = String.valueOf(in);
-				} else if (e.base == null && Character.isWhitespace(in)) {
+				} else if (e.section == null && Character.isWhitespace(in)) {
 					// Save the leading whitespace (if any).
 					if (e.prefix == null) {
 						e.prefix = "";
 					}
 					e.prefix += in;
 				} else if ('[' == in) {
-					// This is a group header line.
-					e.base = readBase(r);
+					// This is a section header.
+					e.section = readSectionName(r);
 					input = r.read();
 					if ('"' == input) {
-						e.extendedBase = readValue(r, true, '"');
+						e.subsection = readValue(r, true, '"');
 						input = r.read();
 					}
 					if (']' != input) {
@@ -770,10 +770,10 @@ public void load() throws IOException {
 					e.suffix = "";
 				} else if (last != null) {
 					// Read a value.
-					e.base = last.base;
-					e.extendedBase = last.extendedBase;
+					e.section = last.section;
+					e.subsection = last.subsection;
 					r.reset();
-					e.name = readName(r);
+					e.name = readKeyName(r);
 					if (e.name.endsWith("\n")) {
 						e.name = e.name.substring(0, e.name.length() - 1);
 						e.value = MAGIC_EMPTY_VALUE;
@@ -809,11 +809,11 @@ protected void clear() {
 	@SuppressWarnings("unchecked")
 	private void add(final Entry e) {
 		entries.add(e);
-		if (e.base != null) {
-			final String b = e.base.toLowerCase();
+		if (e.section != null) {
+			final String b = e.section.toLowerCase();
 			final String group;
-			if (e.extendedBase != null) {
-				group = b + "." + e.extendedBase;
+			if (e.subsection != null) {
+				group = b + "." + e.subsection;
 			} else {
 				group = b;
 			}
@@ -835,8 +835,9 @@ private void add(final Entry e) {
 		}
 	}
 
-	private static String readBase(final BufferedReader r) throws IOException {
-		final StringBuffer base = new StringBuffer();
+	private static String readSectionName(final BufferedReader r)
+			throws IOException {
+		final StringBuffer name = new StringBuffer();
 		for (;;) {
 			r.mark(1);
 			int c = r.read();
@@ -857,22 +858,23 @@ private static String readBase(final BufferedReader r) throws IOException {
 					} else if (' ' == c || '\t' == c) {
 						// Skipped...
 					} else {
-						throw new IOException("Bad base entry. : " + base + ","
-								+ c);
+						throw new IOException("Bad section entry. : " + name
+								+ "," + c);
 					}
 				}
 				break;
 			} else if (Character.isLetterOrDigit((char) c) || '.' == c
 					|| '-' == c) {
-				base.append((char) c);
+				name.append((char) c);
 			} else {
-				throw new IOException("Bad base entry. : " + base + ", " + c);
+				throw new IOException("Bad section entry. : " + name + ", " + c);
 			}
 		}
-		return base.toString();
+		return name.toString();
 	}
 
-	private static String readName(final BufferedReader r) throws IOException {
+	private static String readKeyName(final BufferedReader r)
+			throws IOException {
 		final StringBuffer name = new StringBuffer();
 		for (;;) {
 			r.mark(1);
@@ -1001,12 +1003,12 @@ private static String readValue(final BufferedReader r, boolean quote,
 		/**
 		 * The section name for the entry
 		 */
-		String base;
+		String section;
 
 		/**
 		 * Subsection name
 		 */
-		String extendedBase;
+		String subsection;
 
 		/**
 		 * The key name
@@ -1023,10 +1025,10 @@ private static String readValue(final BufferedReader r, boolean quote,
 		 */
 		String suffix;
 
-		boolean match(final String aBase, final String aExtendedBase,
-				final String aName) {
-			return eq(base, aBase) && eq(extendedBase, aExtendedBase)
-					&& eq(name, aName);
+		boolean match(final String aSection, final String aSubsection,
+				final String aKey) {
+			return eq(section, aSection) && eq(subsection, aSubsection)
+					&& eq(name, aKey);
 		}
 
 		private static boolean eq(final String a, final String b) {
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 08/12] Don't subclass PrintWriter when writing the Config

[Shawn O. Pearce]

Instead of subclassing PrintWriter to override the println() method
we can replace the one println() call we do make with print('\n'),
which ensures we only produce an LF and never a CRLF file.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/Config.java           |    4 ++--
 .../src/org/spearce/jgit/lib/FileBasedConfig.java  |    7 +------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index a2934a2..8eb2e2a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -677,7 +677,7 @@ private int findSectionEnd(final String section, final String subsection) {
 	 * Print configuration file to the PrintWriter
 	 *
 	 * @param r
-	 *             the print writer (it must use '\n' as new line separator).
+	 *             stream to write the configuration to.
 	 */
 	protected void printConfig(final PrintWriter r) {
 		final Iterator<Entry> i = entries.iterator();
@@ -714,7 +714,7 @@ protected void printConfig(final PrintWriter r) {
 			if (e.suffix != null) {
 				r.print(e.suffix);
 			}
-			r.println();
+			r.print('\n');
 		}
 	}
 
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java
index 75e88f6..aa1dbee 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/FileBasedConfig.java
@@ -80,12 +80,7 @@ public void save() throws IOException {
 				+ ".lock");
 		final PrintWriter r = new PrintWriter(new BufferedWriter(
 				new OutputStreamWriter(new FileOutputStream(tmp),
-						Constants.CHARSET))) {
-			@Override
-			public void println() {
-				print('\n');
-			}
-		};
+						Constants.CHARSET)));
 		boolean ok = false;
 		try {
 			printConfig(r);
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 09/12] Use a Java 5 style iteration over the Config entries list

[Shawn O. Pearce]

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index 8eb2e2a..e379c37 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -50,7 +50,6 @@
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -680,9 +679,7 @@ private int findSectionEnd(final String section, final String subsection) {
 	 *             stream to write the configuration to.
 	 */
 	protected void printConfig(final PrintWriter r) {
-		final Iterator<Entry> i = entries.iterator();
-		while (i.hasNext()) {
-			final Entry e = i.next();
+		for (final Entry e : entries) {
 			if (e.prefix != null) {
 				r.print(e.prefix);
 			}
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 10/12] Match config subsection names using case sensitive search

[Shawn O. Pearce]

The subsection name is case sensitive, and should be matched as such.

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index e379c37..974ffea 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -4,6 +4,7 @@
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
  * Copyright (C) 2008, Thad Hughes <thadh@thad.corp.google.com>
  * Copyright (C) 2009, JetBrains s.r.o.
+ * Copyright (C) 2009, Google, Inc.
  *
  * All rights reserved.
  *
@@ -1024,17 +1025,25 @@ private static String readValue(final BufferedReader r, boolean quote,
 
 		boolean match(final String aSection, final String aSubsection,
 				final String aKey) {
-			return eq(section, aSection) && eq(subsection, aSubsection)
-					&& eq(name, aKey);
+			return eqIgnoreCase(section, aSection)
+					&& eqSameCase(subsection, aSubsection)
+					&& eqIgnoreCase(name, aKey);
 		}
 
-		private static boolean eq(final String a, final String b) {
+		private static boolean eqIgnoreCase(final String a, final String b) {
 			if (a == null && b == null)
 				return true;
 			if (a == null || b == null)
 				return false;
 			return a.equalsIgnoreCase(b);
 		}
-	}
 
-}
\ No newline at end of file
+		private static boolean eqSameCase(final String a, final String b) {
+			if (a == null && b == null)
+				return true;
+			if (a == null || b == null)
+				return false;
+			return a.equals(b);
+		}
+	}
+}
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 11/12] Cleanup Config's MAGIC_EMPTY_VALUE to be more safe

[Shawn O. Pearce]

The magic value "%%magic%%empty%%" is just too magic; if it ever
did appear as a value in a key string Config would have treated
it as a true value instead of as a string value.  We also had to
special case conversions of it to the empty string in a string
context.  Instead we create a special String object using the
empty string as a template, and use reference equality against
that to indicate the magic empty value.

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

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index 974ffea..e4528b1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -76,9 +76,14 @@
 	private Map<String, Object> byName;
 
 	/**
-	 * Magic value indicating a missing entry
+	 * Magic value indicating a missing entry.
+	 * <p>
+	 * This value is tested for reference equality in some contexts, so we
+	 * must ensure it is a special copy of the empty string.  It also must
+	 * be treated like the empty string.
 	 */
-	private static final String MAGIC_EMPTY_VALUE = "%%magic%%empty%%";
+	private static final String MAGIC_EMPTY_VALUE = new StringBuilder(0)
+			.toString();
 
 	/**
 	 * The constructor for configuration file
@@ -293,7 +298,7 @@ public boolean getBoolean(final String section, String subsection,
 		if (n == null)
 			return defaultValue;
 
-		if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n)
+		if (MAGIC_EMPTY_VALUE == n || "yes".equalsIgnoreCase(n)
 				|| "true".equalsIgnoreCase(n) || "1".equals(n)
 				|| "on".equalsIgnoreCase(n)) {
 			return true;
@@ -321,11 +326,7 @@ public boolean getBoolean(final String section, String subsection,
 	 */
 	public String getString(final String section, String subsection,
 			final String name) {
-		String val = getRawString(section, subsection, name);
-		if (MAGIC_EMPTY_VALUE.equals(val)) {
-			return "";
-		}
-		return val;
+		return getRawString(section, subsection, name);
 	}
 
 	/**
@@ -345,16 +346,13 @@ public String getString(final String section, String subsection,
 		if (o instanceof List) {
 			final List lst = (List) o;
 			final String[] r = new String[lst.size()];
-			for (int i = 0; i < r.length; i++) {
-				final String val = ((Entry) lst.get(i)).value;
-				r[i] = MAGIC_EMPTY_VALUE.equals(val) ? "" : val;
-			}
+			for (int i = 0; i < r.length; i++)
+				r[i] = ((Entry) lst.get(i)).value;
 			return r;
 		}
 
 		if (o instanceof Entry) {
-			final String val = ((Entry) o).value;
-			return new String[] { MAGIC_EMPTY_VALUE.equals(val) ? "" : val };
+			return new String[] { ((Entry) o).value };
 		}
 
 		if (baseConfig != null)
@@ -700,7 +698,7 @@ protected void printConfig(final PrintWriter r) {
 				}
 				r.print(e.name);
 				if (e.value != null) {
-					if (!MAGIC_EMPTY_VALUE.equals(e.value)) {
+					if (MAGIC_EMPTY_VALUE != e.value) {
 						r.print(" = ");
 						r.print(escapeValue(e.value));
 					}
-- 
1.6.4.rc1.186.g60aa0c

[JGIT PATCH 12/12] Remove unreferenced REMOTE_SECTION from RepositoryConfig

[Shawn O. Pearce]

All callers should be using a RemoteConfig instead.

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

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 9d2c9a3..85e8738 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -66,10 +66,6 @@ public static RepositoryConfig openUserConfig() {
 		return systemReader.openUserConfig();
 	}
 
-
-	/** Section name for a remote configuration */
-	public static final String REMOTE_SECTION = "remote";
-
 	/** Section name for a branch configuration. */
 	public static final String BRANCH_SECTION = "branch";
 
-- 
1.6.4.rc1.186.g60aa0c

Re: [JGIT PATCH 11/12] Cleanup Config's MAGIC_EMPTY_VALUE to be more safe

[Robin Rosenberg]

tisdag 21 juli 2009 22:19:29 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> The magic value "%%magic%%empty%%" is just too magic; if it ever
> did appear as a value in a key string Config would have treated
> it as a true value instead of as a string value.  We also had to
> special case conversions of it to the empty string in a string
> context.  Instead we create a special String object using the
> empty string as a template, and use reference equality against
> that to indicate the magic empty value.
> 
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>  .../src/org/spearce/jgit/lib/Config.java           |   28 +++++++++----------
>  1 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> index 974ffea..e4528b1 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> @@ -76,9 +76,14 @@
>  	private Map<String, Object> byName;
>  
>  	/**
> -	 * Magic value indicating a missing entry
> +	 * Magic value indicating a missing entry.
> +	 * <p>
> +	 * This value is tested for reference equality in some contexts, so we
> +	 * must ensure it is a special copy of the empty string.  It also must
> +	 * be treated like the empty string.
>  	 */
> -	private static final String MAGIC_EMPTY_VALUE = "%%magic%%empty%%";
> +	private static final String MAGIC_EMPTY_VALUE = new StringBuilder(0)
> +			.toString();

Can we be sure an implementation doesn't "optimize" toString() here? But an
explicit new String() shouldn't be..?

-- robin



>  	/**
>  	 * The constructor for configuration file
> @@ -293,7 +298,7 @@ public boolean getBoolean(final String section, String subsection,
>  		if (n == null)
>  			return defaultValue;
>  
> -		if (MAGIC_EMPTY_VALUE.equals(n) || "yes".equalsIgnoreCase(n)
> +		if (MAGIC_EMPTY_VALUE == n || "yes".equalsIgnoreCase(n)
>  				|| "true".equalsIgnoreCase(n) || "1".equals(n)
>  				|| "on".equalsIgnoreCase(n)) {
>  			return true;
> @@ -321,11 +326,7 @@ public boolean getBoolean(final String section, String subsection,
>  	 */
>  	public String getString(final String section, String subsection,
>  			final String name) {
> -		String val = getRawString(section, subsection, name);
> -		if (MAGIC_EMPTY_VALUE.equals(val)) {
> -			return "";
> -		}
> -		return val;
> +		return getRawString(section, subsection, name);
>  	}
>  
>  	/**
> @@ -345,16 +346,13 @@ public String getString(final String section, String subsection,
>  		if (o instanceof List) {
>  			final List lst = (List) o;
>  			final String[] r = new String[lst.size()];
> -			for (int i = 0; i < r.length; i++) {
> -				final String val = ((Entry) lst.get(i)).value;
> -				r[i] = MAGIC_EMPTY_VALUE.equals(val) ? "" : val;
> -			}
> +			for (int i = 0; i < r.length; i++)
> +				r[i] = ((Entry) lst.get(i)).value;
>  			return r;
>  		}
>  
>  		if (o instanceof Entry) {
> -			final String val = ((Entry) o).value;
> -			return new String[] { MAGIC_EMPTY_VALUE.equals(val) ? "" : val };
> +			return new String[] { ((Entry) o).value };
>  		}
>  
>  		if (baseConfig != null)
> @@ -700,7 +698,7 @@ protected void printConfig(final PrintWriter r) {
>  				}
>  				r.print(e.name);
>  				if (e.value != null) {
> -					if (!MAGIC_EMPTY_VALUE.equals(e.value)) {
> +					if (MAGIC_EMPTY_VALUE != e.value) {
>  						r.print(" = ");
>  						r.print(escapeValue(e.value));
>  					}

Re: [JGIT PATCH 11/12] Cleanup Config's MAGIC_EMPTY_VALUE to be more safe

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> tisdag 21 juli 2009 22:19:29 skrev "Shawn O. Pearce" <spearce@spearce.org>:
> > The magic value "%%magic%%empty%%" is just too magic; 
...
> > -	private static final String MAGIC_EMPTY_VALUE = "%%magic%%empty%%";
> > +	private static final String MAGIC_EMPTY_VALUE = new StringBuilder(0)
> > +			.toString();
> 
> Can we be sure an implementation doesn't "optimize" toString() here? But an
> explicit new String() shouldn't be..?

*sigh*

I used this form because FindBugs dislikes new String("").  But yea,
at least with new String() we know its a new string object and
there is no pooling of the empty string going on under the covers.

I can't imagine it being a common enough case to convert a
StringBuilder to an empty string that a runtime implementation would
waste instructions over checking for it and reusing a pooled copy,
but eh, yea, maybe this should be new String("") and include a
FindBugs entry to disable the warning here.

-- 
Shawn.

Re: [JGIT PATCH 10/12] Match config subsection names using case sensitive search

[Constantine Plotnikov]

This patch is incomplete. The method getRawEntry(...) and
setStringList(...) should be fixed as part of this patch too. There is
subsection is converted to lowercase. I was planning to submit it as
separate patch.

Also I'm somewhat bothered by usage of toLowerCase() without locale
specified and equalsIgnoreCase(). When turkish locale is default one
there could be surprising results with the letter "I".  The program:

import java.util.Locale;
public class Test {
	public static void main(String[] args) {
		Locale tr_TR = new Locale("tr", "TR");
		System.out.printf("i = U+%04x LC(I, tr_TR) = U+%04x\n", (int)'i',
(int)"I".toLowerCase(tr_TR).charAt(0));
		System.out.printf("I = U+%04x UC(i, tr_TR) = U+%04x\n", (int)'I',
(int)"i".toUpperCase(tr_TR).charAt(0));
	}
}

Gives the following output:

i = U+0069 LC(I, tr_TR) = U+0131
I = U+0049 UC(i, tr_TR) = U+0130

So I suggest to explicitly use Locale.US for all toLowerCase()
invocation in Config class just in case and to replace
equalsIgnoreCase() with something else. But this possibly should be
some other patch series. I do not know what C git doing in case
turkish locale and whether it is a bug or "feature".

Regards,
Constantine

On Wed, Jul 22, 2009 at 12:19 AM, Shawn O. Pearce<spearce@spearce.org> wrote:
> The subsection name is case sensitive, and should be matched as such.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>  .../src/org/spearce/jgit/lib/Config.java           |   19 ++++++++++++++-----
>  1 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> index e379c37..974ffea 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
> @@ -4,6 +4,7 @@
>  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
>  * Copyright (C) 2008, Thad Hughes <thadh@thad.corp.google.com>
>  * Copyright (C) 2009, JetBrains s.r.o.
> + * Copyright (C) 2009, Google, Inc.
>  *
>  * All rights reserved.
>  *
> @@ -1024,17 +1025,25 @@ private static String readValue(final BufferedReader r, boolean quote,
>
>                boolean match(final String aSection, final String aSubsection,
>                                final String aKey) {
> -                       return eq(section, aSection) && eq(subsection, aSubsection)
> -                                       && eq(name, aKey);
> +                       return eqIgnoreCase(section, aSection)
> +                                       && eqSameCase(subsection, aSubsection)
> +                                       && eqIgnoreCase(name, aKey);
>                }
>
> -               private static boolean eq(final String a, final String b) {
> +               private static boolean eqIgnoreCase(final String a, final String b) {
>                        if (a == null && b == null)
>                                return true;
>                        if (a == null || b == null)
>                                return false;
>                        return a.equalsIgnoreCase(b);
>                }
> -       }
>
> -}
> \ No newline at end of file
> +               private static boolean eqSameCase(final String a, final String b) {
> +                       if (a == null && b == null)
> +                               return true;
> +                       if (a == null || b == null)
> +                               return false;
> +                       return a.equals(b);
> +               }
> +       }
> +}
> --
> 1.6.4.rc1.186.g60aa0c
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [JGIT PATCH 10/12] Match config subsection names using case sensitive search

[Robin Rosenberg]

onsdag 22 juli 2009 13:11:07 skrev Constantine Plotnikov <constantine.plotnikov@gmail.com>:
> This patch is incomplete. The method getRawEntry(...) and
> setStringList(...) should be fixed as part of this patch too. There is
> subsection is converted to lowercase. I was planning to submit it as
> separate patch.
> 
> Also I'm somewhat bothered by usage of toLowerCase() without locale
> specified and equalsIgnoreCase(). When turkish locale is default one
> there could be surprising results with the letter "I".  The program:
> 
> import java.util.Locale;
> public class Test {
> 	public static void main(String[] args) {
> 		Locale tr_TR = new Locale("tr", "TR");
> 		System.out.printf("i = U+%04x LC(I, tr_TR) = U+%04x\n", (int)'i',
> (int)"I".toLowerCase(tr_TR).charAt(0));
> 		System.out.printf("I = U+%04x UC(i, tr_TR) = U+%04x\n", (int)'I',
> (int)"i".toUpperCase(tr_TR).charAt(0));
> 	}
> }
> 
> Gives the following output:
> 
> i = U+0069 LC(I, tr_TR) = U+0131
> I = U+0049 UC(i, tr_TR) = U+0130
> 
> So I suggest to explicitly use Locale.US for all toLowerCase()
> invocation in Config class just in case and to replace
> equalsIgnoreCase() with something else. But this possibly should be
> some other patch series. I do not know what C git doing in case
> turkish locale and whether it is a bug or "feature".

Good catch. C Git operates in "C" locale (default for C/C++) so it should
not be a problem. We might have other places where this could be a problem.
Using Locale.US should be the right choice here since unicode is not allowed
in sections and keys anyway.  (Btw, java.lang.String.regionMatches has some info
on case insensitive matching for those interested, serving a reminder for
why home-brewed logic isn't always the best.)

As you say we should make this a separate patch.

--  robin

[PATCH] Ensure Config readers handle case insensitive names correctly

[Shawn O. Pearce]

In some locales (e.g. Turkish) String toLowerCase() and
equalsIgnoreCase() can produce different results.  For example,
in that locale:

  i = U+0069   LC(I, tr_TR) = U+0131
  I = U+0049   UC(i, tr_TR) = U+0130

C Git and network protocols prefer to operate in the "C" locale,
which does not translate to different codepoints like this, but
instead stays within the US-ASCII character set, U+0069 and U+0049.
When reading a Git configuration file in a Turkish locale we should
honor the US-ASCII interpretation as C Git would, rather than the
native Turkish locale's rules.

By using our own toLowerCase and equalsIgnoreCase implementations we
can ensure we always use the US-ASCII (aka "C" locale) definition of
this 'i' <-> 'I' translation.  A custom table based translation is
used instead of relying on Locale.US to avoid accidental translation
of codepoints > U+007f, as these are not typically translated in the
"C" locale but might still be translated in Java's en_US locale.

CC: Constantine Plotnikov <constantine.plotnikov@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
 > onsdag 22 juli 2009 13:11:07 skrev Constantine Plotnikov <constantine.plotnikov@gmail.com>:
 > > Also I'm somewhat bothered by usage of toLowerCase() without locale
 > > specified and equalsIgnoreCase(). When turkish locale is default one
 > > there could be surprising results with the letter "I".  The program:
 ...
 > Good catch.
 ...
 > As you say we should make this a separate patch.
 
 And here is a patch!  :-)
 
 .../tst/org/spearce/jgit/util/StringUtilsTest.java |   78 +++++++++++++
 .../findBugs/FindBugsExcludeFilter.xml             |   18 +++-
 .../src/org/spearce/jgit/lib/Config.java           |   25 +++--
 .../src/org/spearce/jgit/transport/AmazonS3.java   |   15 ++--
 .../org/spearce/jgit/transport/OpenSshConfig.java  |   19 ++--
 .../src/org/spearce/jgit/util/FS_Win32.java        |    2 +-
 .../org/spearce/jgit/util/RawSubStringPattern.java |    2 +-
 .../src/org/spearce/jgit/util/StringUtils.java     |  115 ++++++++++++++++++++
 8 files changed, 243 insertions(+), 31 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/util/StringUtilsTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/util/StringUtils.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/StringUtilsTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/StringUtilsTest.java
new file mode 100644
index 0000000..0710844
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/StringUtilsTest.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+import junit.framework.TestCase;
+
+public class StringUtilsTest extends TestCase {
+	public void testToLowerCaseChar() {
+		assertEquals('a', StringUtils.toLowerCase('A'));
+		assertEquals('z', StringUtils.toLowerCase('Z'));
+
+		assertEquals('a', StringUtils.toLowerCase('a'));
+		assertEquals('z', StringUtils.toLowerCase('z'));
+
+		assertEquals((char) 0, StringUtils.toLowerCase((char) 0));
+		assertEquals((char) 0xffff, StringUtils.toLowerCase((char) 0xffff));
+	}
+
+	public void testToLowerCaseString() {
+		assertEquals("\n abcdefghijklmnopqrstuvwxyz\n", StringUtils
+				.toLowerCase("\n ABCDEFGHIJKLMNOPQRSTUVWXYZ\n"));
+	}
+
+	public void testEqualsIgnoreCase1() {
+		final String a = "FOO";
+		assertTrue(StringUtils.equalsIgnoreCase(a, a));
+	}
+
+	public void testEqualsIgnoreCase2() {
+		assertFalse(StringUtils.equalsIgnoreCase("a", ""));
+	}
+
+	public void testEqualsIgnoreCase3() {
+		assertFalse(StringUtils.equalsIgnoreCase("a", "b"));
+		assertFalse(StringUtils.equalsIgnoreCase("ac", "ab"));
+	}
+
+	public void testEqualsIgnoreCase4() {
+		assertTrue(StringUtils.equalsIgnoreCase("a", "a"));
+		assertTrue(StringUtils.equalsIgnoreCase("A", "a"));
+		assertTrue(StringUtils.equalsIgnoreCase("a", "A"));
+	}
+}
diff --git a/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml b/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml
index 598197c..6809568 100644
--- a/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml
+++ b/org.spearce.jgit/findBugs/FindBugsExcludeFilter.xml
@@ -8,10 +8,20 @@
        <Method name="mmap" />
        <Bug pattern="DM_GC" />
      </Match>
-	 <!-- Silence the construction of our magic String instance. 
-	  -->
+
+	   <!-- Silence the construction of our magic String instance. 
+	     -->
      <Match>
-     	<Class name="org.spearce.jgit.lib.Config" />
-     	<Bug pattern="DM_STRING_VOID_CTOR"/>
+     	 <Class name="org.spearce.jgit.lib.Config" />
+     	 <Bug pattern="DM_STRING_VOID_CTOR"/>
+     </Match>
+
+     <!-- Silence comparison of string by == or !=.  This class is built
+          only to provide compare of string values, we won't make a mistake
+          here with == assuming .equals() style equality.
+       -->
+     <Match>
+       <Class name="org.spearce.jgit.lib.util.StringUtils" />
+       <Bug pattern="ES_COMPARING_PARAMETER_STRING_WITH_EQ" />
      </Match>
 </FindBugsFilter>
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
index fc1a2a3..258dba5 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Config.java
@@ -55,6 +55,8 @@
 import java.util.Map;
 import java.util.Set;
 
+import org.spearce.jgit.util.StringUtils;
+
 /**
  * The configuration file stored in the format similar to the ".git/config"
  * file.
@@ -236,7 +238,7 @@ public long getLong(final String section, String subsection,
 			return defaultValue;
 
 		long mul = 1;
-		switch (Character.toLowerCase(n.charAt(n.length() - 1))) {
+		switch (StringUtils.toLowerCase(n.charAt(n.length() - 1))) {
 		case 'g':
 			mul = GiB;
 			break;
@@ -297,13 +299,16 @@ public boolean getBoolean(final String section, String subsection,
 		if (n == null)
 			return defaultValue;
 
-		if (MAGIC_EMPTY_VALUE == n || "yes".equalsIgnoreCase(n)
-				|| "true".equalsIgnoreCase(n) || "1".equals(n)
-				|| "on".equalsIgnoreCase(n)) {
+		if (MAGIC_EMPTY_VALUE == n || StringUtils.equalsIgnoreCase("yes", n)
+				|| StringUtils.equalsIgnoreCase("true", n)
+				|| StringUtils.equalsIgnoreCase("1", n)
+				|| StringUtils.equalsIgnoreCase("on", n)) {
 			return true;
 
-		} else if ("no".equalsIgnoreCase(n) || "false".equalsIgnoreCase(n)
-				|| "0".equals(n) || "off".equalsIgnoreCase(n)) {
+		} else if (StringUtils.equalsIgnoreCase("no", n)
+				|| StringUtils.equalsIgnoreCase("false", n)
+				|| StringUtils.equalsIgnoreCase("0", n)
+				|| StringUtils.equalsIgnoreCase("off", n)) {
 			return false;
 
 		} else {
@@ -371,7 +376,8 @@ public String getString(final String section, String subsection,
 		ensureLoaded();
 
 		for (final Entry e : entries) {
-			if (section.equalsIgnoreCase(e.section) && e.subsection != null)
+			if (StringUtils.equalsIgnoreCase(section, e.section)
+					&& e.subsection != null)
 				result.add(e.subsection);
 		}
 		if (baseConfig != null)
@@ -441,7 +447,8 @@ private static String concatenateKey(final String section,
 			ss = "." + subsection;
 		else
 			ss = "";
-		return section.toLowerCase() + ss + "." + name.toLowerCase();
+		return StringUtils.toLowerCase(section) + ss + "."
+				+ StringUtils.toLowerCase(name);
 	}
 
 	/**
@@ -1033,7 +1040,7 @@ private static boolean eqIgnoreCase(final String a, final String b) {
 				return true;
 			if (a == null || b == null)
 				return false;
-			return a.equalsIgnoreCase(b);
+			return StringUtils.equalsIgnoreCase(a, b);
 		}
 
 		private static boolean eqSameCase(final String a, final String b) {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java b/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
index 3d8bdca..d27f37d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/AmazonS3.java
@@ -79,6 +79,7 @@
 import org.spearce.jgit.lib.ProgressMonitor;
 import org.spearce.jgit.util.Base64;
 import org.spearce.jgit.util.HttpSupport;
+import org.spearce.jgit.util.StringUtils;
 import org.spearce.jgit.util.TemporaryBuffer;
 import org.xml.sax.Attributes;
 import org.xml.sax.InputSource;
@@ -122,7 +123,7 @@
 	}
 
 	private static boolean isSignedHeader(final String name) {
-		final String nameLC = name.toLowerCase();
+		final String nameLC = StringUtils.toLowerCase(name);
 		return SIGNED_HEADERS.contains(nameLC) || nameLC.startsWith("x-amz-");
 	}
 
@@ -214,13 +215,13 @@ public AmazonS3(final Properties props) {
 		privateKey = new SecretKeySpec(Constants.encodeASCII(secret), HMAC);
 
 		final String pacl = props.getProperty("acl", "PRIVATE");
-		if ("PRIVATE".equalsIgnoreCase(pacl))
+		if (StringUtils.equalsIgnoreCase("PRIVATE", pacl))
 			acl = "private";
-		else if ("PUBLIC".equalsIgnoreCase(pacl))
+		else if (StringUtils.equalsIgnoreCase("PUBLIC", pacl))
 			acl = "public-read";
-		else if ("PUBLIC-READ".equalsIgnoreCase(pacl))
+		else if (StringUtils.equalsIgnoreCase("PUBLIC-READ", pacl))
 			acl = "public-read";
-		else if ("PUBLIC_READ".equalsIgnoreCase(pacl))
+		else if (StringUtils.equalsIgnoreCase("PUBLIC_READ", pacl))
 			acl = "public-read";
 		else
 			throw new IllegalArgumentException("Invalid acl: " + pacl);
@@ -575,7 +576,7 @@ private void authorize(final HttpURLConnection c) throws IOException {
 		for (final Map.Entry<String, List<String>> entry : reqHdr.entrySet()) {
 			final String hdr = entry.getKey();
 			if (isSignedHeader(hdr))
-				sigHdr.put(hdr.toLowerCase(), toCleanString(entry.getValue()));
+				sigHdr.put(StringUtils.toLowerCase(hdr), toCleanString(entry.getValue()));
 		}
 
 		final StringBuilder s = new StringBuilder();
@@ -781,7 +782,7 @@ public void endElement(final String uri, final String name,
 			if ("Key".equals(name))
 				entries.add(data.toString().substring(prefix.length()));
 			else if ("IsTruncated".equals(name))
-				truncated = "true".equalsIgnoreCase(data.toString());
+				truncated = StringUtils.equalsIgnoreCase("true", data.toString());
 			data = null;
 		}
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/OpenSshConfig.java b/org.spearce.jgit/src/org/spearce/jgit/transport/OpenSshConfig.java
index 92a1bfc..acd1a45 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/OpenSshConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/OpenSshConfig.java
@@ -55,6 +55,7 @@
 import org.spearce.jgit.errors.InvalidPatternException;
 import org.spearce.jgit.fnmatch.FileNameMatcher;
 import org.spearce.jgit.util.FS;
+import org.spearce.jgit.util.StringUtils;
 
 /**
  * Simple configuration parser for the OpenSSH ~/.ssh/config file.
@@ -176,7 +177,7 @@ public Host lookup(final String hostName) {
 			final String keyword = parts[0].trim();
 			final String argValue = parts[1].trim();
 
-			if ("Host".equalsIgnoreCase(keyword)) {
+			if (StringUtils.equalsIgnoreCase("Host", keyword)) {
 				current.clear();
 				for (final String pattern : argValue.split("[ \t]")) {
 					final String name = dequote(pattern);
@@ -197,15 +198,15 @@ public Host lookup(final String hostName) {
 				continue;
 			}
 
-			if ("HostName".equalsIgnoreCase(keyword)) {
+			if (StringUtils.equalsIgnoreCase("HostName", keyword)) {
 				for (final Host c : current)
 					if (c.hostName == null)
 						c.hostName = dequote(argValue);
-			} else if ("User".equalsIgnoreCase(keyword)) {
+			} else if (StringUtils.equalsIgnoreCase("User", keyword)) {
 				for (final Host c : current)
 					if (c.user == null)
 						c.user = dequote(argValue);
-			} else if ("Port".equalsIgnoreCase(keyword)) {
+			} else if (StringUtils.equalsIgnoreCase("Port", keyword)) {
 				try {
 					final int port = Integer.parseInt(dequote(argValue));
 					for (final Host c : current)
@@ -214,19 +215,19 @@ public Host lookup(final String hostName) {
 				} catch (NumberFormatException nfe) {
 					// Bad port number. Don't set it.
 				}
-			} else if ("IdentityFile".equalsIgnoreCase(keyword)) {
+			} else if (StringUtils.equalsIgnoreCase("IdentityFile", keyword)) {
 				for (final Host c : current)
 					if (c.identityFile == null)
 						c.identityFile = toFile(dequote(argValue));
-			} else if ("PreferredAuthentications".equalsIgnoreCase(keyword)) {
+			} else if (StringUtils.equalsIgnoreCase("PreferredAuthentications", keyword)) {
 				for (final Host c : current)
 					if (c.preferredAuthentications == null)
 						c.preferredAuthentications = nows(dequote(argValue));
-			} else if ("BatchMode".equalsIgnoreCase(keyword)) {
+			} else if (StringUtils.equalsIgnoreCase("BatchMode", keyword)) {
 				for (final Host c : current)
 					if (c.batchMode == null)
 						c.batchMode = yesno(dequote(argValue));
-			} else if ("StrictHostKeyChecking".equalsIgnoreCase(keyword)) {
+			} else if (StringUtils.equalsIgnoreCase("StrictHostKeyChecking", keyword)) {
 				String value = dequote(argValue);
 				for (final Host c : current)
 					if (c.strictHostKeyChecking == null)
@@ -268,7 +269,7 @@ private static String nows(final String value) {
 	}
 
 	private static Boolean yesno(final String value) {
-		if ("yes".equalsIgnoreCase(value))
+		if (StringUtils.equalsIgnoreCase("yes", value))
 			return Boolean.TRUE;
 		return Boolean.FALSE;
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/FS_Win32.java b/org.spearce.jgit/src/org/spearce/jgit/util/FS_Win32.java
index ef589bc..009b338 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/FS_Win32.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/FS_Win32.java
@@ -50,7 +50,7 @@ public String run() {
 					}
 				});
 		return osDotName != null
-				&& osDotName.toLowerCase().indexOf("windows") != -1;
+				&& StringUtils.toLowerCase(osDotName).indexOf("windows") != -1;
 	}
 
 	public boolean supportsExecute() {
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java b/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java
index a81bf7f..5ed071c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/RawSubStringPattern.java
@@ -117,7 +117,7 @@ private static final boolean neq(final byte a, final byte b) {
 	}
 
 	private static final byte lc(final byte q) {
-		return (byte) Character.toLowerCase((char) (q & 0xff));
+		return (byte) StringUtils.toLowerCase((char) (q & 0xff));
 	}
 
 	/**
diff --git a/org.spearce.jgit/src/org/spearce/jgit/util/StringUtils.java b/org.spearce.jgit/src/org/spearce/jgit/util/StringUtils.java
new file mode 100644
index 0000000..81d293a
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/util/StringUtils.java
@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2009, Google Inc.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.spearce.jgit.util;
+
+/** Miscellaneous string comparison utility methods. */
+public final class StringUtils {
+	private static final char[] LC;
+
+	static {
+		LC = new char['Z' + 1];
+		for (char c = 0; c < LC.length; c++)
+			LC[c] = c;
+		for (char c = 'A'; c <= 'Z'; c++)
+			LC[c] = (char) ('a' + (c - 'A'));
+	}
+
+	/**
+	 * Convert the input to lowercase.
+	 * <p>
+	 * This method does not honor the JVM locale, but instead always behaves as
+	 * though it is in the US-ASCII locale. Only characters in the range 'A'
+	 * through 'Z' are converted. All other characters are left as-is, even if
+	 * they otherwise would have a lowercase character equivilant.
+	 * 
+	 * @param c
+	 *            the input character.
+	 * @return lowercase version of the input.
+	 */
+	public static char toLowerCase(final char c) {
+		return c <= 'Z' ? LC[c] : c;
+	}
+
+	/**
+	 * Convert the input string to lower case, according to the "C" locale.
+	 * <p>
+	 * This method does not honor the JVM locale, but instead always behaves as
+	 * though it is in the US-ASCII locale. Only characters in the range 'A'
+	 * through 'Z' are converted, all other characters are left as-is, even if
+	 * they otherwise would have a lowercase character equivilant.
+	 * 
+	 * @param in
+	 *            the input string. Must not be null.
+	 * @return a copy of the input string, after converting characters in the
+	 *         range 'A'..'Z' to 'a'..'z'.
+	 */
+	public static String toLowerCase(final String in) {
+		final StringBuilder r = new StringBuilder(in.length());
+		for (int i = 0; i < in.length(); i++)
+			r.append(toLowerCase(in.charAt(i)));
+		return r.toString();
+	}
+
+	/**
+	 * Test if two strings are equal, ignoring case.
+	 * <p>
+	 * This method does not honor the JVM locale, but instead always behaves as
+	 * though it is in the US-ASCII locale.
+	 * 
+	 * @param a
+	 *            first string to compare.
+	 * @param b
+	 *            second string to compare.
+	 * @return true if a equals b
+	 */
+	public static boolean equalsIgnoreCase(final String a, final String b) {
+		if (a == b)
+			return true;
+		if (a.length() != b.length())
+			return false;
+		for (int i = 0; i < a.length(); i++) {
+			if (toLowerCase(a.charAt(i)) != toLowerCase(b.charAt(i)))
+				return false;
+		}
+		return true;
+	}
+
+	private StringUtils() {
+		// Do not create instances
+	}
+}
-- 
1.6.4.rc2.182.g24de1