From 37e3bbc619e8352a642be986d80297db8ec62c7e Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Thu, 21 Dec 2023 18:25:02 +0000 Subject: [PATCH] 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. --- filesystem/squashfs/const_internal_test.go | 16 +++---- filesystem/squashfs/directoryentry.go | 41 ++++++++++++++++- filesystem/squashfs/squashfs.go | 28 ++---------- filesystem/squashfs/squashfs_test.go | 51 ++++++++++++++++++++++ 4 files changed, 103 insertions(+), 33 deletions(-) diff --git a/filesystem/squashfs/const_internal_test.go b/filesystem/squashfs/const_internal_test.go index 0f483630..7b594500 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/directoryentry.go b/filesystem/squashfs/directoryentry.go index 760bcf0a..040212a5 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 294cd6bb..fbfb3c0b 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(), diff --git a/filesystem/squashfs/squashfs_test.go b/filesystem/squashfs/squashfs_test.go index 75c82099..05afdb60 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