Enforce DFS blockLimit is a multiple of blockSize

Change-Id: I2821124ff88d7d1812a846ed20f3828fc9123b38

3 files changed, 31 insertions, 6 deletions

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 45032de9bd..3dd72c3cad 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -41,6 +41,8 @@ bitmapsMustBePrepared=Bitmaps must be prepared before they may be written.
 blameNotCommittedYet=Not Committed Yet
 blobNotFound=Blob not found: {0}
 blobNotFoundForPath=Blob not found: {0} for path: {1}
+blockLimitNotMultipleOfBlockSize=blockLimit {0} must be a multiple of blockSize {1}
+blockLimitNotPositive=blockLimit must be positive: {0}
 blockSizeNotPowerOf2=blockSize must be a power of 2
 bothRefTargetsMustNotBeNull=both old and new ref targets must not be null.
 branchNameInvalid=Branch name {0} is not allowed
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index e23d2534b6..0545a4ecaf 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -102,6 +102,8 @@ public class JGitText extends TranslationBundle {
 	/***/ public String blameNotCommittedYet;
 	/***/ public String blobNotFound;
 	/***/ public String blobNotFoundForPath;
+	/***/ public String blockLimitNotMultipleOfBlockSize;
+	/***/ public String blockLimitNotPositive;
 	/***/ public String blockSizeNotPowerOf2;
 	/***/ public String bothRefTargetsMustNotBeNull;
 	/***/ public String branchNameInvalid;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
index e558a81c09..cd7901b357 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/DfsBlockCacheConfig.java
@@ -95,13 +95,23 @@ public class DfsBlockCacheConfig {
 	/**
 	 * Set maximum number bytes of heap memory to dedicate to caching pack file
 	 * data.
+	 * <p>
+	 * It is strongly recommended to set the block limit to be an integer multiple
+	 * of the block size. This constraint is not enforced by this method (since
+	 * it may be called before {@link #setBlockSize(int)}), but it is enforced by
+	 * {@link #fromConfig(Config)}.
 	 *
 	 * @param newLimit
 	 *            maximum number bytes of heap memory to dedicate to caching
-	 *            pack file data.
+	 *            pack file data; must be positive.
 	 * @return {@code this}
 	 */
 	public DfsBlockCacheConfig setBlockLimit(final long newLimit) {
+		if (newLimit <= 0) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().blockLimitNotPositive,
+					Long.valueOf(newLimit)));
+		}
 		blockLimit = newLimit;
 		return this;
 	}
@@ -188,23 +198,34 @@ public class DfsBlockCacheConfig {
 	 * <p>
 	 * If a property is not defined in the configuration, then it is left
 	 * unmodified.
+	 * <p>
+	 * Enforces certain constraints on the combination of settings in the config,
+	 * for example that the block limit is a multiple of the block size.
 	 *
 	 * @param rc
 	 *            configuration to read properties from.
 	 * @return {@code this}
 	 */
 	public DfsBlockCacheConfig fromConfig(final Config rc) {
-		setBlockLimit(rc.getLong(
+		long cfgBlockLimit = rc.getLong(
 				CONFIG_CORE_SECTION,
 				CONFIG_DFS_SECTION,
 				CONFIG_KEY_BLOCK_LIMIT,
-				getBlockLimit()));
-
-		setBlockSize(rc.getInt(
+				getBlockLimit());
+		int cfgBlockSize = rc.getInt(
 				CONFIG_CORE_SECTION,
 				CONFIG_DFS_SECTION,
 				CONFIG_KEY_BLOCK_SIZE,
-				getBlockSize()));
+				getBlockSize());
+		if (cfgBlockLimit % cfgBlockSize != 0) {
+			throw new IllegalArgumentException(MessageFormat.format(
+					JGitText.get().blockLimitNotMultipleOfBlockSize,
+					Long.valueOf(cfgBlockLimit),
+					Long.valueOf(cfgBlockSize)));
+		}
+
+		setBlockLimit(cfgBlockLimit);
+		setBlockSize(cfgBlockSize);
 
 		setConcurrencyLevel(rc.getInt(
 				CONFIG_CORE_SECTION,