path/filepath: new signature for Walk

This one uses a closure than an interface, and is much simpler to use.
It also enables a called function to return an error and (possibly)
halt processing.

Fixes #2237.

R=golang-dev, gri, rsc, r, cw, n13m3y3r
CC=golang-dev
https://golang.org/cl/5014043

1 files changed, 58 insertions, 51 deletions

diff --git a/src/pkg/path/filepath/path_test.go b/src/pkg/path/filepath/path_test.go
index 9d28992454..850ead8e81 100644
--- a/src/pkg/path/filepath/path_test.go
+++ b/src/pkg/path/filepath/path_test.go
@@ -306,9 +306,9 @@ func makeTree(t *testing.T) {
 
 func markTree(n *Node) { walkTree(n, "", func(path string, n *Node) { n.mark++ }) }
 
-func checkMarks(t *testing.T) {
+func checkMarks(t *testing.T, report bool) {
 	walkTree(tree, tree.name, func(path string, n *Node) {
-		if n.mark != 1 {
+		if n.mark != 1 && report {
 			t.Errorf("node %s mark = %d; expected 1", path, n.mark)
 		}
 		n.mark = 0
@@ -316,44 +316,41 @@ func checkMarks(t *testing.T) {
 }
 
 // Assumes that each node name is unique. Good enough for a test.
-func mark(name string) {
-	name = filepath.ToSlash(name)
+// If clear is true, any incoming error is cleared before return. The errors
+// are always accumulated, though.
+func mark(path string, info *os.FileInfo, err os.Error, errors *[]os.Error, clear bool) os.Error {
+	if err != nil {
+		*errors = append(*errors, err)
+		if clear {
+			return nil
+		}
+		return err
+	}
 	walkTree(tree, tree.name, func(path string, n *Node) {
-		if n.name == name {
+		if n.name == info.Name {
 			n.mark++
 		}
 	})
-}
-
-type TestVisitor struct{}
-
-func (v *TestVisitor) VisitDir(path string, f *os.FileInfo) bool {
-	mark(f.Name)
-	return true
-}
-
-func (v *TestVisitor) VisitFile(path string, f *os.FileInfo) {
-	mark(f.Name)
+	return nil
 }
 
 func TestWalk(t *testing.T) {
 	makeTree(t)
-
-	// 1) ignore error handling, expect none
-	v := &TestVisitor{}
-	filepath.Walk(tree.name, v, nil)
-	checkMarks(t)
-
-	// 2) handle errors, expect none
-	errors := make(chan os.Error, 64)
-	filepath.Walk(tree.name, v, errors)
-	select {
-	case err := <-errors:
+	errors := make([]os.Error, 0, 10)
+	clear := true
+	markFn := func(path string, info *os.FileInfo, err os.Error) os.Error {
+		return mark(path, info, err, &errors, clear)
+	}
+	// Expect no errors.
+	err := filepath.Walk(tree.name, markFn)
+	if err != nil {
 		t.Errorf("no error expected, found: %s", err)
-	default:
-		// ok
 	}
-	checkMarks(t)
+	if len(errors) != 0 {
+		t.Errorf("unexpected errors: %s", errors)
+	}
+	checkMarks(t, true)
+	errors = errors[0:0]
 
 	// Test permission errors.  Only possible if we're not root
 	// and only on some file systems (AFS, FAT).  To avoid errors during
@@ -362,40 +359,50 @@ func TestWalk(t *testing.T) {
 		// introduce 2 errors: chmod top-level directories to 0
 		os.Chmod(filepath.Join(tree.name, tree.entries[1].name), 0)
 		os.Chmod(filepath.Join(tree.name, tree.entries[3].name), 0)
+
+		// 3) capture errors, expect two.
 		// mark respective subtrees manually
 		markTree(tree.entries[1])
 		markTree(tree.entries[3])
 		// correct double-marking of directory itself
 		tree.entries[1].mark--
 		tree.entries[3].mark--
+		err := filepath.Walk(tree.name, markFn)
+		if err != nil {
+			t.Errorf("expected no error return from Walk, %s", err)
+		}
+		if len(errors) != 2 {
+			t.Errorf("expected 2 errors, got %d: %s", len(errors), errors)
+		}
+		// the inaccessible subtrees were marked manually
+		checkMarks(t, true)
+		errors = errors[0:0]
 
-		// 3) handle errors, expect two
-		errors = make(chan os.Error, 64)
-		os.Chmod(filepath.Join(tree.name, tree.entries[1].name), 0)
-		filepath.Walk(tree.name, v, errors)
-	Loop:
-		for i := 1; i <= 2; i++ {
-			select {
-			case <-errors:
-				// ok
-			default:
-				t.Errorf("%d. error expected, none found", i)
-				break Loop
-			}
+		// 4) capture errors, stop after first error.
+		// mark respective subtrees manually
+		markTree(tree.entries[1])
+		markTree(tree.entries[3])
+		// correct double-marking of directory itself
+		tree.entries[1].mark--
+		tree.entries[3].mark--
+		clear = false // error will stop processing
+		err = filepath.Walk(tree.name, markFn)
+		if err == nil {
+			t.Errorf("expected error return from Walk")
 		}
-		select {
-		case err := <-errors:
-			t.Errorf("only two errors expected, found 3rd: %v", err)
-		default:
-			// ok
+		if len(errors) != 1 {
+			t.Errorf("expected 1 error, got %d: %s", len(errors), errors)
 		}
 		// the inaccessible subtrees were marked manually
-		checkMarks(t)
+		checkMarks(t, false)
+		errors = errors[0:0]
+
+		// restore permissions
+		os.Chmod(filepath.Join(tree.name, tree.entries[1].name), 0770)
+		os.Chmod(filepath.Join(tree.name, tree.entries[3].name), 0770)
 	}
 
 	// cleanup
-	os.Chmod(filepath.Join(tree.name, tree.entries[1].name), 0770)
-	os.Chmod(filepath.Join(tree.name, tree.entries[3].name), 0770)
 	if err := os.RemoveAll(tree.name); err != nil {
 		t.Errorf("removeTree: %v", err)
 	}