From 2b0615c76c8acf88de1add9bad1758384fecb9e5 Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Fri, 20 Aug 2021 17:46:45 -0700
Subject: [op] Prevent file leak in MultiGetAttr's error path.

The old implementation was mostly correct but error prone - making way for the
issue in question here. In its error path, it would leak the intermediate file
being walked. Each return/break needed explicit cleanup.

This change implements a more clean way to cleaning up intermediate directories.
If the code were to evolve to be more complex, it would still work.

PiperOrigin-RevId: 392102826
---
 pkg/p9/file.go | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

(limited to 'pkg/p9/file.go')

diff --git a/pkg/p9/file.go b/pkg/p9/file.go
index 97e0231d6..8d6af2d6b 100644
--- a/pkg/p9/file.go
+++ b/pkg/p9/file.go
@@ -325,6 +325,12 @@ func (*DisallowServerCalls) Renamed(File, string) {
 func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
 	stats := make([]FullStat, 0, len(names))
 	parent := start
+	closeParent := func() {
+		if parent != start {
+			_ = parent.Close()
+		}
+	}
+	defer closeParent()
 	mask := AttrMaskAll()
 	for i, name := range names {
 		if len(name) == 0 && i == 0 {
@@ -340,15 +346,14 @@ func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
 			continue
 		}
 		qids, child, valid, attr, err := parent.WalkGetAttr([]string{name})
-		if parent != start {
-			_ = parent.Close()
-		}
 		if err != nil {
 			if errors.Is(err, unix.ENOENT) {
 				return stats, nil
 			}
 			return nil, err
 		}
+		closeParent()
+		parent = child
 		stats = append(stats, FullStat{
 			QID:   qids[0],
 			Valid: valid,
@@ -357,13 +362,8 @@ func DefaultMultiGetAttr(start File, names []string) ([]FullStat, error) {
 		if attr.Mode.FileType() != ModeDirectory {
 			// Doesn't need to continue if entry is not a dir. Including symlinks
 			// that cannot be followed.
-			_ = child.Close()
 			break
 		}
-		parent = child
-	}
-	if parent != start {
-		_ = parent.Close()
 	}
 	return stats, nil
 }
-- 
cgit v1.2.3