Skip to content

Commit

Permalink
Fixed linter issues
Browse files Browse the repository at this point in the history
Code changes to satisfy linters:

 - Ran `gofmt -s -w` on repo.
 - Broke up long lines.
 - When possible, changed names with incorrect initialism formatting
   - Added exceptions for exported variables.
 - Added exceptions for ALL_CAPS_WITH_UNDERSCORES code.
   - Switched to using `windows` or `syscall` definitions if possible;
     especially if some constants were unused.
 - Added `_ =` to satisfy error linter, and acknowledge that errors are
   being ignored.
 - Switched to using `errors.Is` and `As` in places, elsewhere added
   exceptions if error value was known to be `syscall.Errno`.
 - Removed bare returns.
 - Prevented variables from being overshadowed in certain places
   (ignoring cases of overshadowing `err`).
 - Renamed variables and functions (eg, `len`, `eventMetadata.bytes`) to
   prevent shadowing pre-built functions and imported pacakges.
 - Removed unused method receivers.
 - Added exceptions to certain unused (unexported) constants and
   functions.
   - Deleted unused `once` from `pkg/etw.providerMap`.
 - Renamed `noop.go` files to `main_other.go` or `doc.go`, to better fit
   style recommendations.
 - Added exceptions for non-secure use of SHA1 and weak crypto
   libraries.
 - Replaced `ioutil` with `io` and `os` (and `t.TempDir` in tests).
 - Added fully exhaustive checks for `switch` statements in `pkg/etw`.
 - Defined constant strings for `tools/mkwinsyscall`.
 - Removed unnecessary conversions.
 - Made sure `context.Cancel` was called.

Additionally, added `//go:build windows" constraints on files with
unexported code, since linter will complain about unused code on
non-Windows platforms.

Added a stub `main() {}` for `mkwinsyscall` for non-Windows builds, just in
case `//go:generate` directives are added to OS-agnostic files.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
  • Loading branch information
helsaawy committed Aug 15, 2022
1 parent f619a9f commit 57d377b
Show file tree
Hide file tree
Showing 60 changed files with 824 additions and 567 deletions.
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 //revive:disable-line:var-naming ID, not Id
BackupReparseData
BackupSparseBlock
BackupTxfsData
Expand All @@ -35,23 +36,25 @@ const (
StreamSparseAttributes = uint32(8)
)

//nolint:revive // var-naming: 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 {
//revive:disable-next-line:var-naming ID, not 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
23 changes: 11 additions & 12 deletions backup_test.go
Expand Up @@ -5,7 +5,6 @@ package winio

import (
"io"
"io/ioutil"
"os"
"syscall"
"testing"
Expand All @@ -14,7 +13,7 @@ import (
var testFileName string

func TestMain(m *testing.M) {
f, err := ioutil.TempFile("", "tmp")
f, err := os.CreateTemp("", "tmp")
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -62,7 +61,7 @@ func TestBackupRead(t *testing.T) {
defer f.Close()
r := NewBackupFileReader(f, false)
defer r.Close()
b, err := ioutil.ReadAll(r)
b, err := io.ReadAll(r)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -90,7 +89,7 @@ func TestBackupStreamRead(t *testing.T) {
gotAltData := false
for {
hdr, err := br.Next()
if err == io.EOF {
if err == io.EOF { //nolint:errorlint
break
}
if err != nil {
Expand All @@ -105,7 +104,7 @@ func TestBackupStreamRead(t *testing.T) {
if hdr.Name != "" {
t.Fatalf("unexpected name %s", hdr.Name)
}
b, err := ioutil.ReadAll(br)
b, err := io.ReadAll(br)
if err != nil {
t.Fatal(err)
}
Expand All @@ -120,7 +119,7 @@ func TestBackupStreamRead(t *testing.T) {
if hdr.Name != ":ads.txt:$DATA" {
t.Fatalf("incorrect name %s", hdr.Name)
}
b, err := ioutil.ReadAll(br)
b, err := io.ReadAll(br)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -176,15 +175,15 @@ func TestBackupStreamWrite(t *testing.T) {

f.Close()

b, err := ioutil.ReadFile(testFileName)
b, err := os.ReadFile(testFileName)
if err != nil {
t.Fatal(err)
}
if string(b) != data {
t.Fatalf("wrong data %v", b)
}

b, err = ioutil.ReadFile(testFileName + ":ads.txt")
b, err = os.ReadFile(testFileName + ":ads.txt")
if err != nil {
t.Fatal(err)
}
Expand All @@ -202,11 +201,11 @@ func makeSparseFile() error {
defer f.Close()

const (
FSCTL_SET_SPARSE = 0x000900c4
FSCTL_SET_ZERO_DATA = 0x000980c8
fsctlSetSparse = 0x000900c4
fsctlSetZeroData = 0x000980c8
)

err = syscall.DeviceIoControl(syscall.Handle(f.Fd()), FSCTL_SET_SPARSE, nil, 0, nil, 0, nil, nil)
err = syscall.DeviceIoControl(syscall.Handle(f.Fd()), fsctlSetSparse, nil, 0, nil, 0, nil, nil)
if err != nil {
return err
}
Expand Down Expand Up @@ -246,7 +245,7 @@ func TestBackupSparseFile(t *testing.T) {
br := NewBackupStreamReader(r)
for {
hdr, err := br.Next()
if err == io.EOF {
if err == io.EOF { //nolint:errorlint
break
}
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion backuptar/noop.go → backuptar/doc.go
@@ -1,4 +1,3 @@
// +build !windows
// This file only exists to allow go get on non-Windows platforms.

package backuptar
2 changes: 2 additions & 0 deletions backuptar/strconv.go
@@ -1,3 +1,5 @@
//go:build windows

package backuptar

import (
Expand Down

0 comments on commit 57d377b

Please sign in to comment.