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 <dagood@microsoft.com>

* 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 <dagood@microsoft.com>

---------

Signed-off-by: Davis Goodin <dagood@microsoft.com>
  • Loading branch information
dagood authored Jan 16, 2024
1 parent bc421d9 commit 008bc6e
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 4 deletions.
22 changes: 18 additions & 4 deletions fileinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,18 @@ type FileBasicInfo struct {
_ 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{}
bi := &alignedFileBasicInfo{}
if err := windows.GetFileInformationByHandleEx(
windows.Handle(f.Fd()),
windows.FileBasicInfo,
Expand All @@ -30,16 +39,21 @@ func GetFileBasicInfo(f *os.File) (*FileBasicInfo, error) {
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 {
// 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(bi)),
uint32(unsafe.Sizeof(*bi)),
(*byte)(unsafe.Pointer(&biAligned)),
uint32(unsafe.Sizeof(biAligned)),
); err != nil {
return &os.PathError{Op: "SetFileInformationByHandle", Path: f.Name(), Err: err}
}
Expand Down
52 changes: 52 additions & 0 deletions fileinfo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package winio
import (
"os"
"testing"
"unsafe"

"golang.org/x/sys/windows"
)
Expand Down Expand Up @@ -131,3 +132,54 @@ func TestGetFileStandardInfo_Directory(t *testing.T) {
}
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 008bc6e

Please sign in to comment.