From 72bd193edf53a3c2873d8575a362a6058988fbe7 Mon Sep 17 00:00:00 2001
From: Colin Cross <ccross@android.com>
Date: Mon, 16 Mar 2015 00:13:59 -0700
Subject: [PATCH] Fix a bug in spliceModules and add tests

spliceModules was carefully reallocating the slice if necessary, and
then throwing away the reallocated slice and writing to the too-short
original slice.  Fix the bug, and add tests that would have caught it.

Change-Id: Ifc13b2025198b270c97097fd7d28cd36e460c98c
---
 context.go             |   6 +-
 splice_modules_test.go | 133 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 136 insertions(+), 3 deletions(-)
 create mode 100644 splice_modules_test.go

diff --git a/context.go b/context.go
index cee434c..e348ec4 100644
--- a/context.go
+++ b/context.go
@@ -1479,12 +1479,12 @@ func spliceModulesAtIndex(modules []*moduleInfo, i int, newModules []*moduleInfo
 	}
 
 	// Move the end of the slice over by spliceSize-1
-	copy(modules[i+spliceSize:], modules[i+1:])
+	copy(dest[i+spliceSize:], modules[i+1:])
 
 	// Copy the new modules into the slice
-	copy(modules[i:], newModules)
+	copy(dest[i:], newModules)
 
-	return modules
+	return dest
 }
 
 func (c *Context) initSpecialVariables() {
diff --git a/splice_modules_test.go b/splice_modules_test.go
new file mode 100644
index 0000000..05cd81c
--- /dev/null
+++ b/splice_modules_test.go
@@ -0,0 +1,133 @@
+// Copyright 2015 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package blueprint
+
+import (
+	"reflect"
+	"testing"
+)
+
+var (
+	testModuleA = &moduleInfo{variantName: "testModuleA"}
+	testModuleB = &moduleInfo{variantName: "testModuleB"}
+	testModuleC = &moduleInfo{variantName: "testModuleC"}
+	testModuleD = &moduleInfo{variantName: "testModuleD"}
+	testModuleE = &moduleInfo{variantName: "testModuleE"}
+	testModuleF = &moduleInfo{variantName: "testModuleF"}
+)
+
+func (m *moduleInfo) String() string {
+	return m.variantName
+}
+
+var spliceModulesTestCases = []struct {
+	in         []*moduleInfo
+	replace    *moduleInfo
+	with       []*moduleInfo
+	out        []*moduleInfo
+	reallocate bool
+}{
+	{
+		// Insert at the beginning
+		in:         []*moduleInfo{testModuleA, testModuleB, testModuleC},
+		replace:    testModuleA,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC},
+		reallocate: true,
+	},
+	{
+		// Insert in the middle
+		in:         []*moduleInfo{testModuleA, testModuleB, testModuleC},
+		replace:    testModuleB,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC},
+		reallocate: true,
+	},
+	{
+		// Insert at the end
+		in:         []*moduleInfo{testModuleA, testModuleB, testModuleC},
+		replace:    testModuleC,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleA, testModuleB, testModuleD, testModuleE},
+		reallocate: true,
+	},
+	{
+		// Insert over a single element
+		in:         []*moduleInfo{testModuleA},
+		replace:    testModuleA,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleD, testModuleE},
+		reallocate: true,
+	},
+	{
+		// Insert at the beginning without reallocating
+		in:         []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3],
+		replace:    testModuleA,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleD, testModuleE, testModuleB, testModuleC},
+		reallocate: false,
+	},
+	{
+		// Insert in the middle without reallocating
+		in:         []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3],
+		replace:    testModuleB,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleA, testModuleD, testModuleE, testModuleC},
+		reallocate: false,
+	},
+	{
+		// Insert at the end without reallocating
+		in:         []*moduleInfo{testModuleA, testModuleB, testModuleC, nil}[0:3],
+		replace:    testModuleC,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleA, testModuleB, testModuleD, testModuleE},
+		reallocate: false,
+	},
+	{
+		// Insert over a single element without reallocating
+		in:         []*moduleInfo{testModuleA, nil}[0:1],
+		replace:    testModuleA,
+		with:       []*moduleInfo{testModuleD, testModuleE},
+		out:        []*moduleInfo{testModuleD, testModuleE},
+		reallocate: false,
+	},
+}
+
+func TestSpliceModules(t *testing.T) {
+	for _, testCase := range spliceModulesTestCases {
+		in := make([]*moduleInfo, len(testCase.in), cap(testCase.in))
+		copy(in, testCase.in)
+		origIn := in
+		got := spliceModules(in, testCase.replace, testCase.with)
+		if !reflect.DeepEqual(got, testCase.out) {
+			t.Errorf("test case: %v, %v -> %v", testCase.in, testCase.replace, testCase.with)
+			t.Errorf("incorrect output:")
+			t.Errorf("  expected: %v", testCase.out)
+			t.Errorf("       got: %v", got)
+		}
+		if sameArray(origIn, got) != !testCase.reallocate {
+			t.Errorf("test case: %v, %v -> %v", testCase.in, testCase.replace, testCase.with)
+			not := ""
+			if !testCase.reallocate {
+				not = " not"
+			}
+			t.Errorf("  expected to%s reallocate", not)
+		}
+	}
+}
+
+func sameArray(a, b []*moduleInfo) bool {
+	return &a[0:cap(a)][cap(a)-1] == &b[0:cap(b)][cap(b)-1]
+}
\ No newline at end of file