From 1b30a3be2b9a7774083b11ae1b37625e9cf42530 Mon Sep 17 00:00:00 2001
From: Harshavardhana <harsha@minio.io>
Date: Tue, 31 Jan 2017 15:34:49 -0800
Subject: [PATCH] xl/utils: getPartSizeFromIdx should return error. (#3669)

---
 cmd/xl-v1-object.go     | 18 ++++++++++++++++--
 cmd/xl-v1-utils.go      | 21 ++++++++++++++-------
 cmd/xl-v1-utils_test.go | 28 ++++++++++++++++++++++++++--
 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/cmd/xl-v1-object.go b/cmd/xl-v1-object.go
index 86563d362..d25c41d2e 100644
--- a/cmd/xl-v1-object.go
+++ b/cmd/xl-v1-object.go
@@ -542,7 +542,11 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io.
 
 		// Calculate the size of the current part, if size is unknown, curPartSize wil be unknown too.
 		// allowEmptyPart will always be true if this is the first part and false otherwise.
-		curPartSize := getPartSizeFromIdx(size, globalPutPartSize, partIdx)
+		var curPartSize int64
+		curPartSize, err = getPartSizeFromIdx(size, globalPutPartSize, partIdx)
+		if err != nil {
+			return ObjectInfo{}, toObjectErr(err, bucket, object)
+		}
 
 		// Prepare file for eventual optimization in the disk
 		if curPartSize > 0 {
@@ -593,7 +597,17 @@ func (xl xlObjects) PutObject(bucket string, object string, size int64, data io.
 
 		// If we didn't write anything or we know that the next part doesn't have any
 		// data to write, we should quit this loop immediately
-		if partSizeWritten == 0 || getPartSizeFromIdx(size, globalPutPartSize, partIdx+1) == 0 {
+		if partSizeWritten == 0 {
+			break
+		}
+
+		// Check part size for the next index.
+		var partSize int64
+		partSize, err = getPartSizeFromIdx(size, globalPutPartSize, partIdx+1)
+		if err != nil {
+			return ObjectInfo{}, toObjectErr(err, bucket, object)
+		}
+		if partSize == 0 {
 			break
 		}
 	}
diff --git a/cmd/xl-v1-utils.go b/cmd/xl-v1-utils.go
index 536eae968..4a40fdae7 100644
--- a/cmd/xl-v1-utils.go
+++ b/cmd/xl-v1-utils.go
@@ -17,6 +17,7 @@
 package cmd
 
 import (
+	"errors"
 	"hash/crc32"
 	"path"
 	"sync"
@@ -344,29 +345,35 @@ func getOrderedDisks(distribution []int, disks []StorageAPI) (orderedDisks []Sto
 	return orderedDisks
 }
 
+// Errors specifically generated by getPartSizeFromIdx function.
+var (
+	errPartSizeZero  = errors.New("Part size cannot be zero")
+	errPartSizeIndex = errors.New("Part index cannot be smaller than 1")
+)
+
 // getPartSizeFromIdx predicts the part size according to its index. It also
 // returns -1 when totalSize is also -1.
-func getPartSizeFromIdx(totalSize int64, partSize int64, partIndex int) int64 {
+func getPartSizeFromIdx(totalSize int64, partSize int64, partIndex int) (int64, error) {
 	if partSize == 0 {
-		panic("Part size cannot be nil.")
+		return 0, traceError(errPartSizeZero)
 	}
 	if partIndex < 1 {
-		panic("Part index should be greater than 1.")
+		return 0, traceError(errPartSizeIndex)
 	}
 	switch totalSize {
 	case -1, 0:
-		return totalSize
+		return totalSize, nil
 	}
 	// Compute the total count of parts
 	partsCount := totalSize/partSize + 1
 	// Return the part's size
 	switch {
 	case int64(partIndex) < partsCount:
-		return partSize
+		return partSize, nil
 	case int64(partIndex) == partsCount:
 		// Size of last part
-		return totalSize % partSize
+		return totalSize % partSize, nil
 	default:
-		return 0
+		return 0, nil
 	}
 }
diff --git a/cmd/xl-v1-utils_test.go b/cmd/xl-v1-utils_test.go
index 4b80ffbb4..bf6f2205b 100644
--- a/cmd/xl-v1-utils_test.go
+++ b/cmd/xl-v1-utils_test.go
@@ -353,9 +353,33 @@ func TestGetPartSizeFromIdx(t *testing.T) {
 	}
 
 	for i, testCase := range testCases {
-		s := getPartSizeFromIdx(testCase.totalSize, testCase.partSize, testCase.partIndex)
-		if s != testCase.expectedSize {
+		s, err := getPartSizeFromIdx(testCase.totalSize, testCase.partSize, testCase.partIndex)
+		if err != nil {
+			t.Errorf("Test %d: Expected to pass but failed. %s", i+1, err)
+		}
+		if err == nil && s != testCase.expectedSize {
 			t.Errorf("Test %d: The calculated part size is incorrect: expected = %d, found = %d\n", i+1, testCase.expectedSize, s)
 		}
 	}
+
+	testCasesFailure := []struct {
+		totalSize int64
+		partSize  int64
+		partIndex int
+		err       error
+	}{
+		// partSize is 0, error.
+		{10, 0, 1, errPartSizeZero},
+		{10, 1, 0, errPartSizeIndex},
+	}
+
+	for i, testCaseFailure := range testCasesFailure {
+		_, err := getPartSizeFromIdx(testCaseFailure.totalSize, testCaseFailure.partSize, testCaseFailure.partIndex)
+		if err == nil {
+			t.Errorf("Test %d: Expected to failed but passed. %s", i+1, err)
+		}
+		if err != nil && errorCause(err) != testCaseFailure.err {
+			t.Errorf("Test %d: Expected err %s, but got %s", i+1, testCaseFailure.err, errorCause(err))
+		}
+	}
 }