Repository: diskfs/go-diskfs
Pull request #204 (2023-12-23 18:13:36)

squashfs: factor read only OpenFile into Open method on directory entry
This enables us to open the file directly from the directory listing which saves doing another directory traversal to find it again.

This uses a simple `Open` method modeled on the one in the zip archive [File.Open](https://pkg.go.dev/archive/zip#File.Open)

When extracting all the entries from the archive this makes a significant performance improvement as re-reading all the directory entries from the root is quite expensive.



Patch:
diff --git a/filesystem/squashfs/directoryentry.go b/filesystem/squashfs/directoryentry.go
index 760bcf0..040212a 100644
--- a/filesystem/squashfs/directoryentry.go
+++ b/filesystem/squashfs/directoryentry.go
@@ -5,6 +5,8 @@ import (
 	"io/fs"
 	"os"
 	"time"
+
+	"github.com/diskfs/go-diskfs/filesystem"
 )
 
 // FileStat is the extended data underlying a single file, similar to https://golang.org/pkg/syscall/#Stat_t
@@ -21,7 +23,7 @@ type FileStat = *directoryEntry
 //	IsDir() bool        // abbreviation for Mode().IsDir()
 //	Sys() interface{}   // underlying data source (can return nil)
 type directoryEntry struct {
-	isSubdirectory bool
+	fs             *FileSystem // the FileSystem this entry is part of
 	name           string
 	size           int64
 	modTime        time.Time
@@ -30,6 +32,7 @@ type directoryEntry struct {
 	uid            uint32
 	gid            uint32
 	xattrs         map[string]string
+	isSubdirectory bool
 }
 
 func (d *directoryEntry) equal(o *directoryEntry) bool {
@@ -158,3 +161,39 @@ func (d *directoryEntry) Readlink() (string, error) {
 	}
 	return target, nil
 }
+
+// Open returns an filesystem.File from which you can read the
+// contents of a file.
+//
+// Calling this on anything but a file will return an error.
+//
+// Calling this Open method is more efficient than calling
+// FileSystem.OpenFile as it doesn't have to find the file by
+// traversing the directory entries first.
+func (d *directoryEntry) Open() (filesystem.File, error) {
+	// get the inode data for this file
+	// now open the file
+	// get the inode for the file
+	var eFile *extendedFile
+	in := d.inode
+	iType := in.inodeType()
+	body := in.getBody()
+	//nolint:exhaustive // all other cases fall under default
+	switch iType {
+	case inodeBasicFile:
+		extFile := body.(*basicFile).toExtended()
+		eFile = &extFile
+	case inodeExtendedFile:
+		eFile, _ = body.(*extendedFile)
+	default:
+		return nil, fmt.Errorf("inode is of type %d, neither basic nor extended file", iType)
+	}
+
+	return &File{
+		extendedFile: eFile,
+		isReadWrite:  false,
+		isAppend:     false,
+		offset:       0,
+		filesystem:   d.fs,
+	}, nil
+}
diff --git a/filesystem/squashfs/squashfs.go b/filesystem/squashfs/squashfs.go
index 294cd6b..fbfb3c0 100644
--- a/filesystem/squashfs/squashfs.go
+++ b/filesystem/squashfs/squashfs.go
@@ -307,30 +307,9 @@ func (fs *FileSystem) OpenFile(p string, flag int) (filesystem.File, error) {
 		if targetEntry == nil {
 			return nil, fmt.Errorf("target file %s does not exist", p)
 		}
-		// get the inode data for this file
-		// now open the file
-		// get the inode for the file
-		var eFile *extendedFile
-		in := targetEntry.inode
-		iType := in.inodeType()
-		body := in.getBody()
-		//nolint:exhaustive // all other cases fall under default
-		switch iType {
-		case inodeBasicFile:
-			extFile := body.(*basicFile).toExtended()
-			eFile = &extFile
-		case inodeExtendedFile:
-			eFile, _ = body.(*extendedFile)
-		default:
-			return nil, fmt.Errorf("inode is of type %d, neither basic nor extended directory", iType)
-		}
-
-		f = &File{
-			extendedFile: eFile,
-			isReadWrite:  false,
-			isAppend:     false,
-			offset:       0,
-			filesystem:   fs,
+		f, err = targetEntry.Open()
+		if err != nil {
+			return nil, err
 		}
 	} else {
 		f, err = os.OpenFile(path.Join(fs.workspace, p), flag, 0o644)
@@ -440,6 +419,7 @@ func (fs *FileSystem) hydrateDirectoryEntries(entries []*directoryEntryRaw) ([]*
 			}
 		}
 		fullEntries = append(fullEntries, &directoryEntry{
+			fs:             fs,
 			isSubdirectory: e.isSubdirectory,
 			name:           e.name,
 			size:           body.size(),


Test patch:
diff --git a/filesystem/squashfs/const_internal_test.go b/filesystem/squashfs/const_internal_test.go
index 0f48363..7b59450 100644
--- a/filesystem/squashfs/const_internal_test.go
+++ b/filesystem/squashfs/const_internal_test.go
@@ -220,14 +220,14 @@ func testGetFilesystemRoot() []*directoryEntry {
 	// data taken from reading the bytes of the file SquashfsUncompressedfile
 	modTime := time.Unix(0x5c20d8d7, 0)
 	return []*directoryEntry{
-		{true, "foo", 9949, modTime, 0o755, nil, 0, 0, map[string]string{}},
-		{true, "zero", 32, modTime, 0o755, nil, 0, 0, map[string]string{}},
-		{true, "random", 32, modTime, 0o755, nil, 0, 0, map[string]string{}},
-		{false, "emptylink", 0, modTime, 0o777, nil, 0, 0, map[string]string{}},
-		{false, "goodlink", 0, modTime, 0o777, nil, 0, 0, map[string]string{}},
-		{false, "hardlink", 7, modTime, 0o644, nil, 1, 2, map[string]string{}},
-		{false, "README.md", 7, modTime, 0o644, nil, 1, 2, map[string]string{}},
-		{false, "attrfile", 5, modTime, 0o644, nil, 0, 0, map[string]string{"abc": "def", "myattr": "hello"}},
+		{isSubdirectory: true, name: "foo", size: 9949, modTime: modTime, mode: 0o755},
+		{isSubdirectory: true, name: "zero", size: 32, modTime: modTime, mode: 0o755},
+		{isSubdirectory: true, name: "random", size: 32, modTime: modTime, mode: 0o755},
+		{isSubdirectory: false, name: "emptylink", size: 0, modTime: modTime, mode: 0o777},
+		{isSubdirectory: false, name: "goodlink", size: 0, modTime: modTime, mode: 0o777},
+		{isSubdirectory: false, name: "hardlink", size: 7, modTime: modTime, mode: 0o644, uid: 1, gid: 2},
+		{isSubdirectory: false, name: "README.md", size: 7, modTime: modTime, mode: 0o644, uid: 1, gid: 2},
+		{isSubdirectory: false, name: "attrfile", size: 5, modTime: modTime, mode: 0o644, xattrs: map[string]string{"abc": "def", "myattr": "hello"}},
 	}
 }
 
diff --git a/filesystem/squashfs/squashfs_test.go b/filesystem/squashfs/squashfs_test.go
index 75c8209..05afdb6 100644
--- a/filesystem/squashfs/squashfs_test.go
+++ b/filesystem/squashfs/squashfs_test.go
@@ -263,6 +263,57 @@ func TestSquashfsOpenFile(t *testing.T) {
 	})
 }
 
+// Test the Open method on the directory entry
+func TestSquashfsOpen(t *testing.T) {
+	fs, err := getValidSquashfsFSReadOnly()
+	if err != nil {
+		t.Errorf("Failed to get read-only squashfs filesystem: %v", err)
+	}
+	fis, err := fs.ReadDir("/")
+	if err != nil {
+		t.Errorf("Failed to list squashfs filesystem: %v", err)
+	}
+	var dir = make(map[string]os.FileInfo, len(fis))
+	for _, fi := range fis {
+		dir[fi.Name()] = fi
+	}
+
+	// Check a file
+	fi := dir["README.md"]
+	fix, ok := fi.Sys().(squashfs.FileStat)
+	if !ok {
+		t.Fatal("Wrong type")
+	}
+	fh, err := fix.Open()
+	if err != nil {
+		t.Errorf("Failed to open file: %v", err)
+	}
+	gotBuf, err := io.ReadAll(fh)
+	if err != nil {
+		t.Errorf("Failed to read file: %v", err)
+	}
+	err = fh.Close()
+	if err != nil {
+		t.Errorf("Failed to close file: %v", err)
+	}
+	wantBuf := "README\n"
+	if string(gotBuf) != wantBuf {
+		t.Errorf("Expecting to read %q from file but read %q", wantBuf, string(gotBuf))
+	}
+
+	// Check a directory
+	fi = dir["foo"]
+	fix, ok = fi.Sys().(squashfs.FileStat)
+	if !ok {
+		t.Fatal("Wrong type")
+	}
+	_, err = fix.Open()
+	wantErr := fmt.Errorf("inode is of type 8, neither basic nor extended file")
+	if err.Error() != wantErr.Error() {
+		t.Errorf("Want error %q but got %q", wantErr, err)
+	}
+}
+
 func TestSquashfsRead(t *testing.T) {
 	tests := []struct {
 		blocksize  int64