From 7b3dcc31ebf31382458e69356a120cf95fc0b791 Mon Sep 17 00:00:00 2001
From: Colin Cross <ccross@android.com>
Date: Thu, 24 Jan 2019 13:14:39 -0800
Subject: [PATCH] Avoid filepath.Abs

filepath.Abs is surprisingly expensive, it calls os.Getwd every
time, which involves multiple syscalls, a lock, and and allocations.
Use IsAbs and prefix matching instead.

Test: paths_test.go
Change-Id: Ia6cf34d6bef24c694702af1e7a6ff08ffd2d822b
---
 android/paths.go      | 26 +++++-------------
 android/paths_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 20 deletions(-)

diff --git a/android/paths.go b/android/paths.go
index 13b31c789..4b84c97c2 100644
--- a/android/paths.go
+++ b/android/paths.go
@@ -503,16 +503,9 @@ func safePathForSource(ctx PathContext, pathComponents ...string) (SourcePath, e
 		return ret, err
 	}
 
-	abs, err := filepath.Abs(ret.String())
-	if err != nil {
-		return ret, err
-	}
-	buildroot, err := filepath.Abs(ctx.Config().buildDir)
-	if err != nil {
-		return ret, err
-	}
-	if strings.HasPrefix(abs, buildroot) {
-		return ret, fmt.Errorf("source path %s is in output", abs)
+	// absolute path already checked by validateSafePath
+	if strings.HasPrefix(ret.String(), ctx.Config().buildDir) {
+		return ret, fmt.Errorf("source path %s is in output", ret.String())
 	}
 
 	return ret, err
@@ -526,16 +519,9 @@ func pathForSource(ctx PathContext, pathComponents ...string) (SourcePath, error
 		return ret, err
 	}
 
-	abs, err := filepath.Abs(ret.String())
-	if err != nil {
-		return ret, err
-	}
-	buildroot, err := filepath.Abs(ctx.Config().buildDir)
-	if err != nil {
-		return ret, err
-	}
-	if strings.HasPrefix(abs, buildroot) {
-		return ret, fmt.Errorf("source path %s is in output", abs)
+	// absolute path already checked by validatePath
+	if strings.HasPrefix(ret.String(), ctx.Config().buildDir) {
+		return ret, fmt.Errorf("source path %s is in output", ret.String())
 	}
 
 	return ret, nil
diff --git a/android/paths_test.go b/android/paths_test.go
index c4332d26b..1ed07347e 100644
--- a/android/paths_test.go
+++ b/android/paths_test.go
@@ -630,3 +630,64 @@ func TestMaybeRel(t *testing.T) {
 		})
 	}
 }
+
+func TestPathForSource(t *testing.T) {
+	testCases := []struct {
+		name     string
+		buildDir string
+		src      string
+		err      string
+	}{
+		{
+			name:     "normal",
+			buildDir: "out",
+			src:      "a/b/c",
+		},
+		{
+			name:     "abs",
+			buildDir: "out",
+			src:      "/a/b/c",
+			err:      "is outside directory",
+		},
+		{
+			name:     "in out dir",
+			buildDir: "out",
+			src:      "out/a/b/c",
+			err:      "is in output",
+		},
+	}
+
+	funcs := []struct {
+		name string
+		f    func(ctx PathContext, pathComponents ...string) (SourcePath, error)
+	}{
+		{"pathForSource", pathForSource},
+		{"safePathForSource", safePathForSource},
+	}
+
+	for _, f := range funcs {
+		t.Run(f.name, func(t *testing.T) {
+			for _, test := range testCases {
+				t.Run(test.name, func(t *testing.T) {
+					testConfig := TestConfig(test.buildDir, nil)
+					ctx := &configErrorWrapper{config: testConfig}
+					_, err := f.f(ctx, test.src)
+					if len(ctx.errors) > 0 {
+						t.Fatalf("unexpected errors %v", ctx.errors)
+					}
+					if err != nil {
+						if test.err == "" {
+							t.Fatalf("unexpected error %q", err.Error())
+						} else if !strings.Contains(err.Error(), test.err) {
+							t.Fatalf("incorrect error, want substring %q got %q", test.err, err.Error())
+						}
+					} else {
+						if test.err != "" {
+							t.Fatalf("missing error %q", test.err)
+						}
+					}
+				})
+			}
+		})
+	}
+}