Add some more test coverage, address feedback on checking intput length

Signed-off-by: Ryan Harper <[email protected]>

linux/raidcontroller.go

@@ -6,6 +6,7 @@ type RAIDControllerType string
 
 const (
 	MegaRAIDControllerType RAIDControllerType = "megaraid"
+	SmartPqiControllerType RAIDControllerType = "smartpqi"
 )
 
 type RAIDController interface {

smartpqi/arcconf.go

@@ -16,17 +16,11 @@ const (
 	noArcConfRC = 127
 )
 
-func ParsePhysicalDeviceString(rawData string) (PhysicalDevice, error) {
-	pd := PhysicalDevice{}
-
-	return pd, nil
-}
-
 func parseLogicalDevices(rawData string) ([]LogicalDevice, error) {
 	logDevs := []LogicalDevice{}
 
-	lines := strings.Split(rawData, "\n\n\n")
-	for _, device := range lines {
+	devices := strings.Split(rawData, "\n\n\n")
+	for _, device := range devices {
 		ldStart := strings.Index(device, "Logical Device number")
 		if ldStart >= 0 {
 			ldRawData := strings.TrimSpace(device[ldStart:])
@@ -66,12 +60,23 @@ func parseLogicalDeviceString(rawData string) (LogicalDevice, error) {
 	}
 	ld := LogicalDevice{}
 
+	if len(rawData) < 1 {
+		return ld, fmt.Errorf("cannot parse an empty string")
+	}
+
 	// break data into first line which has the logical device number and
 	// the remaining data is in mostly key : value pairs
 	lineData := strings.SplitN(rawData, "\n", 2)
+	if len(lineData) < 2 {
+		return ld, fmt.Errorf("expected more than 2 lines of data, found %d", len(lineData))
+	}
 
 	// extract the LogicalDrive ID from the first token
+	// Logical Device number N
 	toks := strings.Split(strings.TrimSpace(lineData[0]), " ")
+	if len(toks) != 4 {
+		return ld, fmt.Errorf("expected 4 tokens in %q, found %d", lineData[0], len(toks))
+	}
 
 	ldID, err := strconv.Atoi(toks[len(toks)-1])
 	if err != nil {
@@ -123,7 +128,7 @@ func parseLogicalDeviceString(rawData string) (LogicalDevice, error) {
 		case toks[0] == "Array": // 2]
 			aID, err := strconv.Atoi(toks[1])
 			if err != nil {
-				return ld, fmt.Errorf("failed to parse ArrayID from  token '%s': %s", toks[1], err)
+				return ld, fmt.Errorf("failed to parse ArrayID from token '%s': %s", toks[1], err)
 			}
 
 			ld.ArrayID = aID
@@ -135,7 +140,7 @@ func parseLogicalDeviceString(rawData string) (LogicalDevice, error) {
 		case toks[0] == "Size": // 1144609 MB]
 			sizeMB, err := strconv.Atoi(strings.Fields(toks[1])[0])
 			if err != nil {
-				return ld, fmt.Errorf("failed to parse Size from  token '%s': %s", toks[1], err)
+				return ld, fmt.Errorf("failed to parse Size from token '%s': %s", toks[1], err)
 			}
 
 			ld.SizeMB = sizeMB
@@ -172,15 +177,21 @@ func parsePhysicalDevices(output string) ([]PhysicalDevice, error) {
 	devStartIdx := []int{}
 	devEndIdx := []int{}
 
-	for idx, devIdx := range deviceStartRe.FindAllIndex([]byte(output), -1) {
+	devStart := deviceStartRe.FindAllIndex([]byte(output), -1)
+	if len(devStart) < 1 {
+		return []PhysicalDevice{}, fmt.Errorf("error finding start of PhysicalDevice in data")
+	}
+
+	// construct pairs of start and stop points in the string marking the
+	// beginning and end of a single PhysicalDevice entry
+	for idx, devIdx := range devStart {
 		devStartIdx = append(devStartIdx, devIdx[0])
 
 		if idx > 0 {
 			// 0, 1
 			devEndIdx = append(devEndIdx, devIdx[0]-1)
 		}
 	}
-
 	devEndIdx = append(devEndIdx, len(output))
 
 	deviceRaw := []string{}
@@ -191,6 +202,10 @@ func parsePhysicalDevices(output string) ([]PhysicalDevice, error) {
 
 	for _, deviceStr := range deviceRaw {
 		deviceLines := strings.SplitN(deviceStr, "\n", 2)
+		if len(deviceLines) < 2 {
+			return []PhysicalDevice{}, fmt.Errorf("expected more than 2 lines of data, found %d", len(deviceRaw))
+		}
+
 		toks := strings.Split(deviceLines[0], "#")
 		pdID, err := strconv.Atoi(toks[1])
 
@@ -230,7 +245,7 @@ func parsePhysicalDevices(output string) ([]PhysicalDevice, error) {
 
 				bSize, err := strconv.Atoi(dToks[0])
 				if err != nil {
-					return []PhysicalDevice{}, fmt.Errorf("failed to parse block size from %q: %s", dToks[0], err)
+					return []PhysicalDevice{}, fmt.Errorf("failed to parse Block Size from token %q: %s", dToks[0], err)
 				}
 
 				pd.BlockSize = bSize
@@ -239,14 +254,14 @@ func parsePhysicalDevices(output string) ([]PhysicalDevice, error) {
 
 				bSize, err := strconv.Atoi(dToks[0])
 				if err != nil {
-					return []PhysicalDevice{}, fmt.Errorf("failed to parse block size from %q: %s", dToks[0], err)
+					return []PhysicalDevice{}, fmt.Errorf("failed to parse Physical Block Size from token %q: %s", dToks[0], err)
 				}
 
 				pd.PhysicalBlockSize = bSize
 			case toks[0] == "Array":
 				aID, err := strconv.Atoi(toks[1])
 				if err != nil {
-					return []PhysicalDevice{}, fmt.Errorf("failed to parse Array ID from %q: %s", toks[1], err)
+					return []PhysicalDevice{}, fmt.Errorf("failed to parse Array from token %q: %s", toks[1], err)
 				}
 
 				pd.ArrayID = aID
@@ -271,7 +286,7 @@ func parsePhysicalDevices(output string) ([]PhysicalDevice, error) {
 				bSize, err := strconv.Atoi(dToks[0])
 
 				if err != nil {
-					return []PhysicalDevice{}, fmt.Errorf("failed to parse total size from %q: %s", dToks[0], err)
+					return []PhysicalDevice{}, fmt.Errorf("failed to parse Total Size from token %q: %s", dToks[0], err)
 				}
 
 				pd.SizeMB = bSize
@@ -293,7 +308,15 @@ func parseGetConf(output string) ([]LogicalDevice, []PhysicalDevice, error) {
 	logDevs := []LogicalDevice{}
 	phyDevs := []PhysicalDevice{}
 
+	if len(output) < 1 {
+		return logDevs, phyDevs, fmt.Errorf("cannot parse an empty string")
+	}
+
 	lines := strings.Split(output, "\n\n\n")
+	if len(lines) < 3 {
+		return logDevs, phyDevs, fmt.Errorf("expected more than 3 lines of data in input")
+	}
+
 	for _, device := range lines {
 		ldStart := strings.Index(device, "Logical Device number")
 		if ldStart >= 0 {
@@ -482,7 +505,7 @@ func newController(cID int, arcGetConfigOut string) (Controller, error) {
 
 	lDevs, pDevs, err := parseGetConf(arcGetConfigOut)
 	if err != nil {
-		return Controller{}, fmt.Errorf("failed to parse acrconf getconfig output: %s", err)
+		return Controller{}, fmt.Errorf("failed to parse arcconf getconfig output: %s", err)
 	}
 
 	// PD.ID -> PhysicalDevice
@@ -494,14 +517,15 @@ func newController(cID int, arcGetConfigOut string) (Controller, error) {
 		pDev := pDevs[idx]
 		ctrl.PhysicalDrives[pDev.ID] = &pDev
 
-		for lIdx := range lDevs {
-			lDev := lDevs[lIdx]
-			ctrl.LogicalDrives[lDev.ID] = &lDev
+	}
 
+	for lIdx := range lDevs {
+		lDev := lDevs[lIdx]
+		ctrl.LogicalDrives[lDev.ID] = &lDev
+		for idx := range pDevs {
+			pDev := pDevs[idx]
 			if lDev.ArrayID == pDev.ArrayID {
 				lDev.Devices = append(lDev.Devices, &pDev)
-
-				break
 			}
 		}
 	}