Skip to content

Commit

Permalink
fileinfo: internally fix FileBasicInfo memory alignment (#312)
Browse files Browse the repository at this point in the history
* fileinfo: internally fix FileBasicInfo memory alignment

Signed-off-by: Davis Goodin <[email protected]>

* Update test with review feedback

Remove unused winName.

Extract more into Windows alignment consts to repeat less.

Document reason for having multiple alignment consts for the same value.

Signed-off-by: Davis Goodin <[email protected]>

---------

Signed-off-by: Davis Goodin <[email protected]>
(cherry picked from commit 008bc6e)
Signed-off-by: Kirtana Ashok <[email protected]>
  • Loading branch information
dagood authored and kiashok committed May 21, 2024
1 parent 7e149e8 commit b48c6a3
Show file tree
Hide file tree
Showing 2 changed files with 216 additions and 4 deletions.
35 changes: 31 additions & 4 deletions fileinfo.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build windows
// +build windows

package winio
Expand All @@ -7,6 +8,8 @@ import (
"runtime"
"syscall"
"unsafe"

"golang.org/x/sys/windows"
)

//sys getFileInformationByHandleEx(h syscall.Handle, class uint32, buffer *byte, size uint32) (err error) = GetFileInformationByHandleEx
Expand All @@ -24,19 +27,43 @@ type FileBasicInfo struct {
pad uint32 // padding
}

// alignedFileBasicInfo is a FileBasicInfo, but aligned to uint64 by containing
// uint64 rather than windows.Filetime. Filetime contains two uint32s. uint64
// alignment is necessary to pass this as FILE_BASIC_INFO.
type alignedFileBasicInfo struct {
CreationTime, LastAccessTime, LastWriteTime, ChangeTime uint64
FileAttributes uint32
_ uint32 // padding
}

// GetFileBasicInfo retrieves times and attributes for a file.
func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) {
bi := &FileBasicInfo{}
if err := getFileInformationByHandleEx(syscall.Handle(f.Fd()), fileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil {
bi := &alignedFileBasicInfo{}
if err := windows.GetFileInformationByHandleEx(
windows.Handle(f.Fd()),
windows.FileBasicInfo,
(*byte)(unsafe.Pointer(bi)),
uint32(unsafe.Sizeof(*bi)),
); err != nil {
return nil, &os.PathError{Op: "GetFileInformationByHandleEx", Path: f.Name(), Err: err}
}
runtime.KeepAlive(f)
return bi, nil
// Reinterpret the alignedFileBasicInfo as a FileBasicInfo so it matches the
// public API of this module. The data may be unnecessarily aligned.
return (*FileBasicInfo)(unsafe.Pointer(bi)), nil
}

// SetFileBasicInfo sets times and attributes for a file.
func SetFileBasicInfo(f *os.File, bi *FileBasicInfo) error {
if err := setFileInformationByHandle(syscall.Handle(f.Fd()), fileBasicInfo, (*byte)(unsafe.Pointer(bi)), uint32(unsafe.Sizeof(*bi))); err != nil {
// Create an alignedFileBasicInfo based on a FileBasicInfo. The copy is
// suitable to pass to GetFileInformationByHandleEx.
biAligned := *(*alignedFileBasicInfo)(unsafe.Pointer(bi))
if err := windows.SetFileInformationByHandle(
windows.Handle(f.Fd()),
windows.FileBasicInfo,
(*byte)(unsafe.Pointer(&biAligned)),
uint32(unsafe.Sizeof(biAligned)),
); err != nil {
return &os.PathError{Op: "SetFileInformationByHandle", Path: f.Name(), Err: err}
}
runtime.KeepAlive(f)
Expand Down
185 changes: 185 additions & 0 deletions fileinfo_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
//go:build windows
// +build windows

package winio

import (
"os"
"testing"
"unsafe"

"golang.org/x/sys/windows"
)

// Checks if current matches expected. Note that AllocationSize is filesystem-specific,
// so we check that the current.AllocationSize is >= expected.AllocationSize.
// https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/5afa7f66-619c-48f3-955f-68c4ece704ae
func checkFileStandardInfo(t *testing.T, current, expected *FileStandardInfo) {
t.Helper()

if current.AllocationSize < expected.AllocationSize {
t.Fatalf("FileStandardInfo unexpectedly had AllocationSize %d, expecting >=%d", current.AllocationSize, expected.AllocationSize)
}

if current.EndOfFile != expected.EndOfFile {
t.Fatalf("FileStandardInfo unexpectedly had EndOfFile %d, expecting %d", current.EndOfFile, expected.EndOfFile)
}

if current.NumberOfLinks != expected.NumberOfLinks {
t.Fatalf("FileStandardInfo unexpectedly had NumberOfLinks %d, expecting %d", current.NumberOfLinks, expected.NumberOfLinks)
}

if current.DeletePending != expected.DeletePending {
if current.DeletePending {
t.Fatalf("FileStandardInfo unexpectedly DeletePending")
} else {
t.Fatalf("FileStandardInfo unexpectedly not DeletePending")
}
}

if current.Directory != expected.Directory {
if current.Directory {
t.Fatalf("FileStandardInfo unexpectedly Directory")
} else {
t.Fatalf("FileStandardInfo unexpectedly not Directory")
}
}
}

func TestGetFileStandardInfo_File(t *testing.T) {
f, err := os.CreateTemp("", "tst")
if err != nil {
t.Fatal(err)
}
defer f.Close()
defer os.Remove(f.Name())

expectedFileInfo := &FileStandardInfo{
AllocationSize: 0,
EndOfFile: 0,
NumberOfLinks: 1,
DeletePending: false,
Directory: false,
}

info, err := GetFileStandardInfo(f)
if err != nil {
t.Fatal(err)
}
checkFileStandardInfo(t, info, expectedFileInfo)

bytesWritten, err := f.Write([]byte("0123456789"))
if err != nil {
t.Fatal(err)
}

expectedFileInfo.EndOfFile = int64(bytesWritten)
expectedFileInfo.AllocationSize = int64(bytesWritten)

info, err = GetFileStandardInfo(f)
if err != nil {
t.Fatal(err)
}
checkFileStandardInfo(t, info, expectedFileInfo)

linkName := f.Name() + ".link"

if err = os.Link(f.Name(), linkName); err != nil {
t.Fatal(err)
}
defer os.Remove(linkName)

expectedFileInfo.NumberOfLinks = 2

info, err = GetFileStandardInfo(f)
if err != nil {
t.Fatal(err)
}
checkFileStandardInfo(t, info, expectedFileInfo)

os.Remove(linkName)

expectedFileInfo.NumberOfLinks = 1

info, err = GetFileStandardInfo(f)
if err != nil {
t.Fatal(err)
}
checkFileStandardInfo(t, info, expectedFileInfo)
}

func TestGetFileStandardInfo_Directory(t *testing.T) {
tempDir := t.TempDir()
// os.Open returns the Search Handle, not the Directory Handle
// See https://github.com/golang/go/issues/13738
f, err := OpenForBackup(tempDir, windows.GENERIC_READ, 0, windows.OPEN_EXISTING)
if err != nil {
t.Fatal(err)
}
defer f.Close()

expectedFileInfo := &FileStandardInfo{
AllocationSize: 0,
EndOfFile: 0,
NumberOfLinks: 1,
DeletePending: false,
Directory: true,
}

info, err := GetFileStandardInfo(f)
if err != nil {
t.Fatal(err)
}
checkFileStandardInfo(t, info, expectedFileInfo)
}

// TestFileInfoStructAlignment checks that the alignment of Go fileinfo structs
// match what is expected by the Windows API.
func TestFileInfoStructAlignment(t *testing.T) {
//nolint:revive // SNAKE_CASE is not idiomatic in Go, but aligned with Win32 API.
const (
// The alignment of various types, as named in the Windows APIs. When
// deciding on an expectedAlignment for a struct's test case, use the
// type of the largest field in the struct as written in the Windows
// docs. This is intended to help reviewers by allowing them to first
// check that a new align* const is correct, then independently check
// that the test case is correct, rather than all at once.
alignLARGE_INTEGER = unsafe.Alignof(uint64(0))
alignULONGLONG = unsafe.Alignof(uint64(0))
)
tests := []struct {
name string
actualAlign uintptr
actualSize uintptr
expectedAlignment uintptr
}{
{
// alignedFileBasicInfo is passed to the Windows API rather than FileBasicInfo.
"alignedFileBasicInfo", unsafe.Alignof(alignedFileBasicInfo{}), unsafe.Sizeof(alignedFileBasicInfo{}),
// https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_basic_info
alignLARGE_INTEGER,
},
{
"FileStandardInfo", unsafe.Alignof(FileStandardInfo{}), unsafe.Sizeof(FileStandardInfo{}),
// https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_standard_info
alignLARGE_INTEGER,
},
{
"FileIDInfo", unsafe.Alignof(FileIDInfo{}), unsafe.Sizeof(FileIDInfo{}),
// https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_info
alignULONGLONG,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.actualAlign != tt.expectedAlignment {
t.Errorf("alignment mismatch: actual %d, expected %d", tt.actualAlign, tt.expectedAlignment)
}
if r := tt.actualSize % tt.expectedAlignment; r != 0 {
t.Errorf(
"size is not a multiple of alignment: size %% alignment (%d %% %d) is %d, expected 0",
tt.actualSize, tt.expectedAlignment, r)
}
})
}
}

0 comments on commit b48c6a3

Please sign in to comment.