From 052f4727fea8fa279d36e6b25e74b325b7de7e19 Mon Sep 17 00:00:00 2001
From: Paul Duffin <paulduffin@google.com>
Date: Fri, 23 Oct 2020 18:26:03 +0100
Subject: [PATCH] Make ConfiguredJarList immutable

By making the Append and RemoveList methods return a new list instead
of modifying the existing list it makes the ConfiguredJarList usages
easier to reason about and safer to use, especially considering that
they are primarily used in global configuration.

Bug: 171479578
Test: m nothing
Change-Id: I102c4fb42f0c54e4ed299d2921fbf5efeb6e99b9
---
 android/config.go        | 38 ++++++++++++++++++++------------------
 java/dexpreopt_config.go |  7 +++----
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/android/config.go b/android/config.go
index 42a286c45..8b5ede6e7 100644
--- a/android/config.go
+++ b/android/config.go
@@ -1362,13 +1362,22 @@ func (l *ConfiguredJarList) IndexOfJar(jar string) int {
 }
 
 // Append an (apex, jar) pair to the list.
-func (l *ConfiguredJarList) Append(apex string, jar string) {
-	l.apexes = append(l.apexes, apex)
-	l.jars = append(l.jars, jar)
+func (l *ConfiguredJarList) Append(apex string, jar string) ConfiguredJarList {
+	// Create a copy of the backing arrays before appending to avoid sharing backing
+	// arrays that are mutated across instances.
+	apexes := make([]string, 0, len(l.apexes)+1)
+	copy(apexes, l.apexes)
+	apexes = append(apexes, apex)
+
+	jars := make([]string, 0, len(l.jars)+1)
+	copy(jars, l.jars)
+	jars = append(l.jars, jar)
+
+	return ConfiguredJarList{apexes, jars}
 }
 
 // Filter out sublist.
-func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) {
+func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) ConfiguredJarList {
 	apexes := make([]string, 0, l.Len())
 	jars := make([]string, 0, l.Len())
 
@@ -1380,13 +1389,7 @@ func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) {
 		}
 	}
 
-	l.apexes = apexes
-	l.jars = jars
-}
-
-// A copy of itself.
-func (l *ConfiguredJarList) CopyOf() ConfiguredJarList {
-	return ConfiguredJarList{CopyOf(l.apexes), CopyOf(l.jars)}
+	return ConfiguredJarList{apexes, jars}
 }
 
 // A copy of the list of strings containing jar components.
@@ -1461,17 +1464,16 @@ func splitConfiguredJarPair(ctx PathContext, str string) (string, string) {
 }
 
 func CreateConfiguredJarList(ctx PathContext, list []string) ConfiguredJarList {
-	apexes := make([]string, 0, len(list))
-	jars := make([]string, 0, len(list))
+	apexes := make([]string, len(list))
+	jars := make([]string, len(list))
 
-	l := ConfiguredJarList{apexes, jars}
-
-	for _, apexjar := range list {
+	for i, apexjar := range list {
 		apex, jar := splitConfiguredJarPair(ctx, apexjar)
-		l.Append(apex, jar)
+		apexes[i] = apex
+		jars[i] = jar
 	}
 
-	return l
+	return ConfiguredJarList{apexes, jars}
 }
 
 func EmptyConfiguredJarList() ConfiguredJarList {
diff --git a/java/dexpreopt_config.go b/java/dexpreopt_config.go
index 0f8888a7d..c315124c9 100644
--- a/java/dexpreopt_config.go
+++ b/java/dexpreopt_config.go
@@ -81,13 +81,12 @@ func genBootImageConfigs(ctx android.PathContext) map[string]*bootImageConfig {
 		targets := dexpreoptTargets(ctx)
 		deviceDir := android.PathForOutput(ctx, ctx.Config().DeviceName())
 
-		artModules := global.ArtApexJars.CopyOf()
+		artModules := global.ArtApexJars
 		// With EMMA_INSTRUMENT_FRAMEWORK=true the Core libraries depend on jacoco.
 		if ctx.Config().IsEnvTrue("EMMA_INSTRUMENT_FRAMEWORK") {
-			artModules.Append("com.android.art", "jacocoagent")
+			artModules = artModules.Append("com.android.art", "jacocoagent")
 		}
-		frameworkModules := global.BootJars.CopyOf()
-		frameworkModules.RemoveList(artModules)
+		frameworkModules := global.BootJars.RemoveList(artModules)
 
 		artSubdir := "apex/art_boot_images/javalib"
 		frameworkSubdir := "system/framework"