Skip to content

Commit

Permalink
Fixing lint issues
Browse files Browse the repository at this point in the history
Various linting fixes:

`socket.runtimeFunc` was re-ordered to satisfy govet:
`struct with 56 pointer bytes could be 16`.
Apparently, moving pointers to the beginning of a struct allows the GC
to quit scanning the struct early, at the last pointer instead of the
last byte.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
  • Loading branch information
helsaawy committed Aug 14, 2022
1 parent 6e077ef commit 7ce5344
Show file tree
Hide file tree
Showing 56 changed files with 687 additions and 441 deletions.
1 change: 1 addition & 0 deletions .gitattributes
@@ -0,0 +1 @@
* text=set eol=lf
32 changes: 23 additions & 9 deletions .github/workflows/ci.yml
Expand Up @@ -4,7 +4,7 @@ on:
- pull_request

env:
GO_VERSION: "1.17"
GO_VERSION: "1.18.5"

jobs:
lint:
Expand All @@ -15,27 +15,43 @@ jobs:
- uses: actions/setup-go@v3
with:
go-version: ${{ env.GO_VERSION }}
cache: true
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v3
with:
args: >-
--verbose
--timeout=8m
--timeout=5m
--config=./.golangci.yml
--max-issues-per-linter=0
--max-same-issues=0
--modules-download-mode=readonly
continue-on-error: true # remove when lint issues are fixed
continue-on-error: true
- name: check issues
shell: powershell
run: |
dir .
& "$(go env GOROOT)\bin\gofmt" -w -d .
git add -N .
git diff --stat
Write-Output "::group::git diff full"
git diff --exit-code
Write-Output "::endgroup::"
if ($LASTEXITCODE -ne 0) {
exit $LASTEXITCODE
}
go-generate:
name: Validate Go Generate
runs-on: "windows-2019"
needs:
- lint
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: ${{ env.GO_VERSION }}
cache: true
- name: Verify generated files
shell: powershell
run: |
Expand All @@ -58,10 +74,10 @@ jobs:
test:
name: Run Tests
runs-on: ${{ matrix.os }}
needs:
- lint
- go-generate
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [windows-2019, windows-2022, ubuntu-latest]
Expand All @@ -70,20 +86,18 @@ jobs:
- uses: actions/setup-go@v3
with:
go-version: ${{ env.GO_VERSION }}
cache: true
- run: go test -gcflags=all=-d=checkptr -v ./...

build:
name: Build Repo
runs-on: "windows-2019"
needs:
- test
runs-on: "windows-2019"
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: ${{ env.GO_VERSION }}
cache: true
- run: go build ./pkg/etw/sample/
- run: go build ./tools/etw-provider-gen/
- run: go build ./tools/mkwinsyscall/
Expand Down
57 changes: 42 additions & 15 deletions .golangci.yml
@@ -1,48 +1,68 @@
run:
timeout: 2m

issues:
max-same-issues: 5
max-issues-per-linter: 100
skip-dirs:
- pkg/etw/sample

linters:
enable:
# style
- containedctx # struct contains a context
- dupl # duplicate code
- errname # erorrs are named correctly
- goconst # strings that should be constants
- lll # long lines
- misspell
- predeclared # shadows predeclared Go identifier
- stylecheck
- revive # golint replacement
- stylecheck # golint replacement
- unconvert # unnecessary conversions
- whitespace # unnecessary leading and trailing new lines

# complexity
- nestif # deeply nested ifs

# bugs and others...
# bugs, performance, unsued, etc ...
- contextcheck # function uses a non-inherited context
- dupl # code clone
- errname # erorrs are named correctly
- errorlint # errors not wrapped for 1.13
- exhaustive # check exhaustiveness of enum switch statements
- exportloopref # using loop variable pointer
- gofmt
- gosec # security
- nestif # deeply nested ifs
- nilerr # returns nil even with non-nil error
- prealloc # slices that can be pre-allocated
- revive # golint replacement
- structcheck # unused struct fields
- unparam # unused function params

issues:
max-same-issues: 5
max-issues-per-linter: 100

exclude-rules:
# err is very often shadowed in nested scopes
- linters:
- govet
text: 'shadow: declaration of "err" shadows declaration'

# skip autogen directives for long lines
- linters:
- lll
source: "^//(go:generate|sys) "


linters-settings:
govet:
enable-all: true
disable:
# struct order is often for Win32 compat
# also, ignore pointer bytes/GC issues for now until performance becomes an issue
- fieldalignment
check-shadowing: true
settings:
shadow:
strict: false
lll:
line-length: 140
stylecheck:
checks:
- all
initialisms:
# defaults (https://staticcheck.io/docs/configuration/options/#initialisms)
# defaults: https://staticcheck.io/docs/configuration/options/#initialisms
- ACL
- API
- ASCII
Expand Down Expand Up @@ -90,20 +110,27 @@ linters-settings:
- CID
- CRI
- CTRD
- DACL
- ETW
- GCS
- GMSA
- HCS
- IO
- FSCTL
- LCOW
- LPAC
- LTSC
- MMIO
- OCI
- PMEM
- RX
- SACl
- SID
- SMB
- TX
- VHD
- VHDX
- VM
- VMID
- VPCI
- WCOW
1 change: 0 additions & 1 deletion .vscode/settings.json
Expand Up @@ -2,7 +2,6 @@
"go.lintTool": "golangci-lint",
"go.lintOnSave": "package",
"go.lintFlags": [
"--fix",
"--verbose",
],
}
47 changes: 28 additions & 19 deletions backup.go
Expand Up @@ -8,11 +8,12 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"runtime"
"syscall"
"unicode/utf16"

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

//sys backupRead(h syscall.Handle, b []byte, bytesRead *uint32, abort bool, processSecurity bool, context *uintptr) (err error) = BackupRead
Expand All @@ -25,7 +26,7 @@ const (
BackupAlternateData
BackupLink
BackupPropertyData
BackupObjectId
BackupObjectId //nolint:stylecheck,revive
BackupReparseData
BackupSparseBlock
BackupTxfsData
Expand All @@ -35,23 +36,25 @@ const (
StreamSparseAttributes = uint32(8)
)

//nolint:revive,stylecheck // var-naming,ST1003: ALL_CAPS
const (
WRITE_DAC = 0x40000
WRITE_OWNER = 0x80000
ACCESS_SYSTEM_SECURITY = 0x1000000
WRITE_DAC = windows.WRITE_DAC
WRITE_OWNER = windows.WRITE_DAC
ACCESS_SYSTEM_SECURITY = windows.ACCESS_SYSTEM_SECURITY
)

// BackupHeader represents a backup stream of a file.
type BackupHeader struct {
//nolint:revive,stylecheck // Id -> ID
Id uint32 // The backup stream ID
Attributes uint32 // Stream attributes
Size int64 // The size of the stream in bytes
Name string // The name of the stream (for BackupAlternateData only).
Offset int64 // The offset of the stream in the file (for BackupSparseBlock only).
}

type win32StreamId struct {
StreamId uint32
type win32StreamID struct {
StreamID uint32
Attributes uint32
Size uint64
NameSize uint32
Expand All @@ -72,7 +75,7 @@ func NewBackupStreamReader(r io.Reader) *BackupStreamReader {
// Next returns the next backup stream and prepares for calls to Read(). It skips the remainder of the current stream if
// it was not completely read.
func (r *BackupStreamReader) Next() (*BackupHeader, error) {
if r.bytesLeft > 0 {
if r.bytesLeft > 0 { //nolint:nestif //todo: flatten this
if s, ok := r.r.(io.Seeker); ok {
// Make sure Seek on io.SeekCurrent sometimes succeeds
// before trying the actual seek.
Expand All @@ -83,16 +86,16 @@ func (r *BackupStreamReader) Next() (*BackupHeader, error) {
r.bytesLeft = 0
}
}
if _, err := io.Copy(ioutil.Discard, r); err != nil {
if _, err := io.Copy(io.Discard, r); err != nil {
return nil, err
}
}
var wsi win32StreamId
var wsi win32StreamID
if err := binary.Read(r.r, binary.LittleEndian, &wsi); err != nil {
return nil, err
}
hdr := &BackupHeader{
Id: wsi.StreamId,
Id: wsi.StreamID,
Attributes: wsi.Attributes,
Size: int64(wsi.Size),
}
Expand All @@ -103,7 +106,7 @@ func (r *BackupStreamReader) Next() (*BackupHeader, error) {
}
hdr.Name = syscall.UTF16ToString(name)
}
if wsi.StreamId == BackupSparseBlock {
if wsi.StreamID == BackupSparseBlock {
if err := binary.Read(r.r, binary.LittleEndian, &hdr.Offset); err != nil {
return nil, err
}
Expand Down Expand Up @@ -148,8 +151,8 @@ func (w *BackupStreamWriter) WriteHeader(hdr *BackupHeader) error {
return fmt.Errorf("missing %d bytes", w.bytesLeft)
}
name := utf16.Encode([]rune(hdr.Name))
wsi := win32StreamId{
StreamId: hdr.Id,
wsi := win32StreamID{
StreamID: hdr.Id,
Attributes: hdr.Attributes,
Size: uint64(hdr.Size),
NameSize: uint32(len(name) * 2),
Expand Down Expand Up @@ -204,7 +207,7 @@ func (r *BackupFileReader) Read(b []byte) (int, error) {
var bytesRead uint32
err := backupRead(syscall.Handle(r.f.Fd()), b, &bytesRead, false, r.includeSecurity, &r.ctx)
if err != nil {
return 0, &os.PathError{"BackupRead", r.f.Name(), err}
return 0, &os.PathError{Op: "BackupRead", Path: r.f.Name(), Err: err}
}
runtime.KeepAlive(r.f)
if bytesRead == 0 {
Expand All @@ -217,7 +220,7 @@ func (r *BackupFileReader) Read(b []byte) (int, error) {
// the underlying file.
func (r *BackupFileReader) Close() error {
if r.ctx != 0 {
backupRead(syscall.Handle(r.f.Fd()), nil, nil, true, false, &r.ctx)
_ = backupRead(syscall.Handle(r.f.Fd()), nil, nil, true, false, &r.ctx)
runtime.KeepAlive(r.f)
r.ctx = 0
}
Expand All @@ -243,7 +246,7 @@ func (w *BackupFileWriter) Write(b []byte) (int, error) {
var bytesWritten uint32
err := backupWrite(syscall.Handle(w.f.Fd()), b, &bytesWritten, false, w.includeSecurity, &w.ctx)
if err != nil {
return 0, &os.PathError{"BackupWrite", w.f.Name(), err}
return 0, &os.PathError{Op: "BackupWrite", Path: w.f.Name(), Err: err}
}
runtime.KeepAlive(w.f)
if int(bytesWritten) != len(b) {
Expand All @@ -256,7 +259,7 @@ func (w *BackupFileWriter) Write(b []byte) (int, error) {
// close the underlying file.
func (w *BackupFileWriter) Close() error {
if w.ctx != 0 {
backupWrite(syscall.Handle(w.f.Fd()), nil, nil, true, false, &w.ctx)
_ = backupWrite(syscall.Handle(w.f.Fd()), nil, nil, true, false, &w.ctx)
runtime.KeepAlive(w.f)
w.ctx = 0
}
Expand All @@ -272,7 +275,13 @@ func OpenForBackup(path string, access uint32, share uint32, createmode uint32)
if err != nil {
return nil, err
}
h, err := syscall.CreateFile(&winPath[0], access, share, nil, createmode, syscall.FILE_FLAG_BACKUP_SEMANTICS|syscall.FILE_FLAG_OPEN_REPARSE_POINT, 0)
h, err := syscall.CreateFile(&winPath[0],
access,
share,
nil,
createmode,
syscall.FILE_FLAG_BACKUP_SEMANTICS|syscall.FILE_FLAG_OPEN_REPARSE_POINT,
0)
if err != nil {
err = &os.PathError{Op: "open", Path: path, Err: err}
return nil, err
Expand Down

0 comments on commit 7ce5344

Please sign in to comment.