tar: minor refactoring of populateTree and flatten

Cleaned up some dead code, and made populateTree not
take in a *tar.Streamer argument, as the ts argument was
only used to set an error. The function now returns
an error (if any). Also made flatten not have to take in
a *tar.Streamer argument as well.

Signed-off-by: Stephen Chung <schung@redhat.com>

tar.go

@@ -67,11 +67,10 @@ func (ts *tarStream) readHeaders() {
 	for {
 		hdr, err := ts.tarReader.Next()
 		if err != nil {
-			flatten(&root, ts)
+			flatten(&root, &ts.creator, ts.keywords)
 			ts.pipeReader.CloseWithError(err)
 			return
 		}
-
 		// Because the content of the file may need to be read by several
 		// KeywordFuncs, it needs to be an io.Seeker as well. So, just reading from
 		// ts.tarReader is not enough.
@@ -117,10 +116,7 @@ func (ts *tarStream) readHeaders() {
 				if hdr.FileInfo().IsDir() && keyword == "size" {
 					continue
 				}
-
+				// TODO: handle hardlinks
-				if string(hdr.Typeflag) == string('1') {
-					// TODO: get number of hardlinks for a file
-				}
 				val, err := keyFunc(hdr.Name, hdr.FileInfo(), tmpFile)
 				if err != nil {
 					ts.setErr(err)
@@ -171,7 +167,10 @@ func (ts *tarStream) readHeaders() {
 				e.Set = &s
 			}
 		}
-		populateTree(&root, &e, hdr, ts)
+		err = populateTree(&root, &e, hdr)
+		if err != nil {
+			ts.setErr(err)
+		}
 		tmpFile.Close()
 		os.Remove(tmpFile.Name())
 	}
@@ -186,16 +185,17 @@ const (
 	parentDir
 )
 
-// populateTree creates a file tree hierarchy using an Entry's Parent and
+// populateTree creates a pseudo file tree hierarchy using an Entry's Parent and
 // Children fields. When examining the Entry e to insert in the tree, we
 // determine if the path to that Entry exists yet. If it does, insert it in the
-// appropriate position in the tree. If not, create a path with "placeholder"
+// appropriate position in the tree. If not, create a path up until the Entry's
-// directories, and then insert the Entry. populateTree does not consider
+// directory that it is contained in. Then, insert the Entry.
-// symbolic links yet.
+// root: the "." Entry
-func populateTree(root, e *Entry, hdr *tar.Header, ts *tarStream) {
+//    e: the Entry we are looking to insert
+//  hdr: the tar header struct associated with e
+func populateTree(root, e *Entry, hdr *tar.Header) error {
 	isDir := hdr.FileInfo().IsDir()
 	wd := filepath.Clean(hdr.Name)
-
 	if !isDir {
 		// If entry is a file, we only want the directory it's in.
 		wd = filepath.Dir(wd)
@@ -207,31 +207,24 @@ func populateTree(root, e *Entry, hdr *tar.Header, ts *tarStream) {
 			root.Children = append([]*Entry{e}, root.Children...)
 			e.Parent = root
 		}
-		return
+		return nil
 	}
-
 	dirNames := strings.Split(wd, "/")
 	parent := root
 	for _, name := range dirNames[1:] {
-		if node := parent.Descend(name); node == nil {
+		encoded, err := Vis(name)
+		if err != nil {
+			return err
+		}
+		if node := parent.Descend(encoded); node == nil {
 			// Entry for directory doesn't exist in tree relative to root
-			var newEntry *Entry
+			newEntry := Entry{
-			if isDir {
+				Name:   encoded,
-				newEntry = e
+				Type:   RelativeType,
-			} else {
+				Parent: parent,
-				encodedName, err := Vis(name)
-				if err != nil {
-					ts.setErr(err)
-					return
-				}
-				newEntry = &Entry{
-					Name: encodedName,
-					Type: RelativeType,
-				}
 			}
-			newEntry.Parent = parent
+			parent.Children = append(parent.Children, &newEntry)
-			parent.Children = append(parent.Children, newEntry)
+			parent = &newEntry
-			parent = newEntry
 		} else {
 			// Entry for directory exists in tree, just keep going
 			parent = node
@@ -241,82 +234,88 @@ func populateTree(root, e *Entry, hdr *tar.Header, ts *tarStream) {
 		parent.Children = append([]*Entry{e}, parent.Children...)
 		e.Parent = parent
 	} else {
-		commentpath, err := e.Path()
+		// the "placeholder" directory already exists in the Entry "parent",
+		// so now we have to replace it's underlying data with that from e,
+		// as well as set the Parent field. Note that we don't set parent = e
+		// because parent is already in the pseudo tree, we just need to
+		// complete it's data.
+		e.Parent = parent.Parent
+		*parent = *e
+		commentpath, err := parent.Path()
 		if err != nil {
-			ts.setErr(err)
+			return err
-			return
 		}
-		commentEntry := Entry{
+		parent.Prev = &Entry{
 			Raw:  "# " + commentpath,
 			Type: CommentType,
 		}
-		e.Prev = &commentEntry
 	}
+	return nil
 }
 
-// After constructing the tree from the tar stream, we want to "flatten" this
+// After constructing a pseudo file hierarchy tree, we want to "flatten" this
-// tree by appending Entry's into ts.creator.DH.Entries in an appropriate
+// tree by putting the Entries into a slice with appropriate positioning.
-// manner to simplify writing the output with ts.creator.DH.WriteTo
 // root: the "head" of the sub-tree to flatten
-// ts  : tarStream to keep track of Entry's
+// creator: a dhCreator that helps with the '/set' keyword
-func flatten(root *Entry, ts *tarStream) {
+// keywords: keywords specified by the user that should be evaluated
+func flatten(root *Entry, creator *dhCreator, keywords []string) {
 	if root.Prev != nil {
 		// root.Prev != nil implies root is a directory
-		ts.creator.DH.Entries = append(ts.creator.DH.Entries,
+		creator.DH.Entries = append(creator.DH.Entries,
 			Entry{
 				Type: BlankType,
-				Pos:  len(ts.creator.DH.Entries),
+				Pos:  len(creator.DH.Entries),
 			})
-		root.Prev.Pos = len(ts.creator.DH.Entries)
+		root.Prev.Pos = len(creator.DH.Entries)
-		ts.creator.DH.Entries = append(ts.creator.DH.Entries, *root.Prev)
+		creator.DH.Entries = append(creator.DH.Entries, *root.Prev)
 
 		// Check if we need a new set
-		if ts.creator.curSet == nil {
+		if creator.curSet == nil {
-			ts.creator.curSet = &Entry{
+			creator.curSet = &Entry{
 				Type:     SpecialType,
 				Name:     "/set",
-				Keywords: keywordSelector(append(tarDefaultSetKeywords, root.Set.Keywords...), ts.keywords),
+				Keywords: keywordSelector(append(tarDefaultSetKeywords, root.Set.Keywords...), keywords),
-				Pos:      len(ts.creator.DH.Entries),
+				Pos:      len(creator.DH.Entries),
 			}
-			ts.creator.DH.Entries = append(ts.creator.DH.Entries, *ts.creator.curSet)
+			creator.DH.Entries = append(creator.DH.Entries, *creator.curSet)
 		} else {
 			needNewSet := false
 			for _, k := range root.Set.Keywords {
-				if !inSlice(k, ts.creator.curSet.Keywords) {
+				if !inSlice(k, creator.curSet.Keywords) {
 					needNewSet = true
 					break
 				}
 			}
 			if needNewSet {
-				ts.creator.curSet = &Entry{
+				creator.curSet = &Entry{
 					Name:     "/set",
 					Type:     SpecialType,
-					Pos:      len(ts.creator.DH.Entries),
+					Pos:      len(creator.DH.Entries),
-					Keywords: keywordSelector(append(tarDefaultSetKeywords, root.Set.Keywords...), ts.keywords),
+					Keywords: keywordSelector(append(tarDefaultSetKeywords, root.Set.Keywords...), keywords),
 				}
-				ts.creator.DH.Entries = append(ts.creator.DH.Entries, *ts.creator.curSet)
+				creator.DH.Entries = append(creator.DH.Entries, *creator.curSet)
 			}
 		}
 	}
-	root.Set = ts.creator.curSet
+	root.Set = creator.curSet
-	root.Keywords = setDifference(root.Keywords, ts.creator.curSet.Keywords)
+	root.Keywords = setDifference(root.Keywords, creator.curSet.Keywords)
-	root.Pos = len(ts.creator.DH.Entries)
+	root.Pos = len(creator.DH.Entries)
-	ts.creator.DH.Entries = append(ts.creator.DH.Entries, *root)
+	creator.DH.Entries = append(creator.DH.Entries, *root)
 
 	for _, c := range root.Children {
-		flatten(c, ts)
+		flatten(c, creator, keywords)
 	}
 
 	if root.Prev != nil {
 		// Show a comment when stepping out
-		root.Prev.Pos = len(ts.creator.DH.Entries)
+		root.Prev.Pos = len(creator.DH.Entries)
-		ts.creator.DH.Entries = append(ts.creator.DH.Entries, *root.Prev)
+		creator.DH.Entries = append(creator.DH.Entries, *root.Prev)
 		dotEntry := Entry{
 			Type: DotDotType,
 			Name: "..",
-			Pos:  len(ts.creator.DH.Entries),
+			Pos:  len(creator.DH.Entries),
 		}
-		ts.creator.DH.Entries = append(ts.creator.DH.Entries, dotEntry)
+		creator.DH.Entries = append(creator.DH.Entries, dotEntry)
 	}
 }
 
@@ -332,7 +331,7 @@ func filter(root *Entry, p func(*Entry) bool) []Entry {
 			}
 			if p(c) {
 				if c.Prev == nil {
-					// prepend directories
+					// prepend files
 					validEntrys = append([]Entry{*c}, validEntrys...)
 				} else {
 					validEntrys = append(validEntrys, *c)