From 925f364a79745fcea410fd0ec417d3f0fd1c2d4c Mon Sep 17 00:00:00 2001 From: satoru Date: Wed, 13 Jan 2021 22:51:46 +0800 Subject: [PATCH] Fix all errors reported by golangci-lint and set up pre-commit hook (#43) Co-authored-by: Davies Liu --- .golangci.yml | 4 ++++ .pre-commit-config.yaml | 11 +++++++++++ CONTRIBUTING.md | 1 + cmd/usage.go | 2 +- pkg/chunk/cached_store.go | 4 ++-- pkg/chunk/cached_store_test.go | 8 ++++++-- pkg/chunk/disk_cache.go | 22 +++++++++++----------- pkg/chunk/disk_cache_test.go | 10 +++++----- pkg/chunk/disk_store.go | 2 +- pkg/chunk/store_test.go | 2 ++ pkg/chunk/utils_unix.go | 2 +- pkg/redis/redis.go | 6 ++++-- pkg/redis/redis_test.go | 19 +++++++++++-------- pkg/vfs/vfs.go | 6 +++--- pkg/vfs/vfs_unix.go | 2 +- pkg/vfs/writer.go | 6 +++++- 16 files changed, 69 insertions(+), 38 deletions(-) create mode 100644 .golangci.yml create mode 100644 .pre-commit-config.yaml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..f99fde3d --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,4 @@ +run: + timeout: 5m + skip-dirs: + - fstests diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..b458cf86 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,11 @@ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v2.3.0 + hooks: + - id: check-yaml + - id: end-of-file-fixer + - id: trailing-whitespace + - repo: https://github.com/golangci/golangci-lint + rev: v1.33.0 + hooks: + - id: golangci-lint diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 395a9c83..c958590f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,7 @@ - Use `go fmt` to format your code before committing. You can find information in editor support for Go tools in ["IDEs and Plugins for Go"](https://github.com/golang/go/wiki/IDEsAndTextEditorPlugins). - If you see any code which clearly violates the style guide, please fix it and send a pull request. - Every new source file must begin with a license header. +- Install [pre-commit](https://pre-commit.com/) and use it to set up a pre-commit hook for static analysis. Just run `pre-commit install` in the root of the repo. ## Sign the CLA diff --git a/cmd/usage.go b/cmd/usage.go index 27d95ca2..1521498a 100644 --- a/cmd/usage.go +++ b/cmd/usage.go @@ -75,7 +75,7 @@ func reportUsage(m meta.Meta) { var start = time.Now() for { var totalSpace, availSpace, iused, iavail uint64 - m.StatFS(ctx, &totalSpace, &availSpace, &iused, &iavail) + _ = m.StatFS(ctx, &totalSpace, &availSpace, &iused, &iavail) u.Uptime = int64(time.Since(start).Seconds()) u.UsedSpace = int64(totalSpace - availSpace) u.UsedInodes = int64(iused) diff --git a/pkg/chunk/cached_store.go b/pkg/chunk/cached_store.go index c9e534bc..7f88be87 100644 --- a/pkg/chunk/cached_store.go +++ b/pkg/chunk/cached_store.go @@ -257,7 +257,7 @@ func (c *wChunk) WriteAt(p []byte, off int64) (n int, err error) { // Fill previous blocks with zeros if c.length < int(off) { zeros := make([]byte, int(off)-c.length) - c.WriteAt(zeros, int64(c.length)) + _, _ = c.WriteAt(zeros, int64(c.length)) } for n < len(p) { @@ -649,7 +649,7 @@ func NewCachedStore(storage object.ObjectStorage, config Config) ChunkStore { } p := NewOffPage(size) defer p.Release() - store.load(key, p, true) + _ = store.load(key, p, true) }) go store.uploadStaging() return store diff --git a/pkg/chunk/cached_store_test.go b/pkg/chunk/cached_store_test.go index 7f7b2257..e2659345 100644 --- a/pkg/chunk/cached_store_test.go +++ b/pkg/chunk/cached_store_test.go @@ -29,7 +29,9 @@ func BenchmarkCachedRead(b *testing.B) { config.BlockSize = 4 << 20 store := NewCachedStore(blob, config) w := store.NewWriter(1) - w.WriteAt(make([]byte, 1024), 0) + if _, err := w.WriteAt(make([]byte, 1024), 0); err != nil { + b.Fatalf("write fail: %s", err) + } if err := w.Finish(1024); err != nil { b.Fatalf("write fail: %s", err) } @@ -51,7 +53,9 @@ func BenchmarkUncachedRead(b *testing.B) { config.CacheSize = 0 store := NewCachedStore(blob, config) w := store.NewWriter(2) - w.WriteAt(make([]byte, 1024), 0) + if _, err := w.WriteAt(make([]byte, 1024), 0); err != nil { + b.Fatalf("write fail: %s", err) + } if err := w.Finish(1024); err != nil { b.Fatalf("write fail: %s", err) } diff --git a/pkg/chunk/disk_cache.go b/pkg/chunk/disk_cache.go index 963ddfe7..ece2f4be 100644 --- a/pkg/chunk/disk_cache.go +++ b/pkg/chunk/disk_cache.go @@ -152,20 +152,20 @@ func (cache *cacheStore) flushPage(path string, data []byte) error { _, err = f.Write(data) if err != nil { logger.Infof("Write to cache file %s: %s", tmp, err) - f.Close() - os.Remove(tmp) + _ = f.Close() + _ = os.Remove(tmp) return err } err = f.Close() if err != nil { logger.Infof("Close cache file %s: %s", tmp, err) - os.Remove(tmp) + _ = os.Remove(tmp) return err } err = os.Rename(tmp, path) if err != nil { logger.Infof("Rename cache file %s -> %s: %s", tmp, path, err) - os.Remove(tmp) + _ = os.Remove(tmp) } return err } @@ -178,9 +178,9 @@ func (cache *cacheStore) createDir(dir string) { if filepath.Dir(dir) != dir { cache.createDir(filepath.Dir(dir)) } - os.Mkdir(dir, mode) + _ = os.Mkdir(dir, mode) // umask may remove some permisssions - os.Chmod(dir, mode) + _ = os.Chmod(dir, mode) } else if strings.HasPrefix(dir, cache.dir) && err == nil && st.Mode() != mode { changeMode(dir, st, mode) } @@ -197,8 +197,8 @@ func (cache *cacheStore) remove(key string) { } cache.Unlock() if path != "" { - os.Remove(path) - os.Remove(cache.stagePath(key)) + _ = os.Remove(path) + _ = os.Remove(cache.stagePath(key)) } } @@ -363,7 +363,7 @@ func (c *cacheStore) scanCached() { cachePrefix := filepath.Join(c.dir, cacheDir) logger.Debugf("Scan %s to find cached blocks", cachePrefix) - filepath.Walk(cachePrefix, func(path string, fi os.FileInfo, err error) error { + _ = filepath.Walk(cachePrefix, func(path string, fi os.FileInfo, err error) error { if fi != nil { if fi.IsDir() || strings.HasSuffix(path, ".tmp") { if fi.ModTime().Before(oneMinAgo) { @@ -401,7 +401,7 @@ func (cache *cacheStore) scanStaging() map[string]string { stagingBlocks := make(map[string]string) stagingPrefix := filepath.Join(cache.dir, stagingDir) logger.Debugf("Scan %s to find staging blocks", stagingPrefix) - filepath.Walk(stagingPrefix, func(path string, fi os.FileInfo, err error) error { + _ = filepath.Walk(stagingPrefix, func(path string, fi os.FileInfo, err error) error { if fi != nil { if fi.IsDir() || strings.HasSuffix(path, ".tmp") { if fi.ModTime().Before(oneMinAgo) { @@ -433,7 +433,7 @@ type cacheManager struct { func keyHash(s string) uint32 { hash := fnv.New32() - hash.Write([]byte(s)) + _, _ = hash.Write([]byte(s)) return hash.Sum32() } diff --git a/pkg/chunk/disk_cache_test.go b/pkg/chunk/disk_cache_test.go index 85d55ed4..b7fca32a 100644 --- a/pkg/chunk/disk_cache_test.go +++ b/pkg/chunk/disk_cache_test.go @@ -28,11 +28,11 @@ func TestExpand(t *testing.T) { t.FailNow() } - os.Mkdir("/tmp/aaa1", 0755) - os.Mkdir("/tmp/aaa2", 0755) - os.Mkdir("/tmp/aaa3", 0755) - os.Mkdir("/tmp/aaa3/jfscache", 0755) - os.Mkdir("/tmp/aaa3/jfscache/jfs", 0755) + _ = os.Mkdir("/tmp/aaa1", 0755) + _ = os.Mkdir("/tmp/aaa2", 0755) + _ = os.Mkdir("/tmp/aaa3", 0755) + _ = os.Mkdir("/tmp/aaa3/jfscache", 0755) + _ = os.Mkdir("/tmp/aaa3/jfscache/jfs", 0755) rs = expandDir("/tmp/aaa*/jfscache/jfs") if len(rs) != 3 || rs[0] != "/tmp/aaa1/jfscache/jfs" { diff --git a/pkg/chunk/disk_store.go b/pkg/chunk/disk_store.go index 21f60545..ff9e8173 100644 --- a/pkg/chunk/disk_store.go +++ b/pkg/chunk/disk_store.go @@ -91,7 +91,7 @@ func (s *diskStore) chunkPath(chunkid uint64) string { func NewDiskStore(dir string) ChunkStore { if _, err := os.Stat(dir); os.IsNotExist(err) { - os.Mkdir(dir, 0755) + _ = os.Mkdir(dir, 0755) } return &diskStore{dir} } diff --git a/pkg/chunk/store_test.go b/pkg/chunk/store_test.go index 32138800..333d9f57 100644 --- a/pkg/chunk/store_test.go +++ b/pkg/chunk/store_test.go @@ -46,6 +46,7 @@ func testStore(t *testing.T, store ChunkStore) { if err := writer.Finish(size); err != nil { t.Fatalf("finish fail: %s", err) } + // nolint:errcheck defer store.Remove(1, size) reader := store.NewReader(1, size) @@ -91,6 +92,7 @@ func TestUncompressedStore(t *testing.T) { testStore(t, store) } +// nolint:errcheck func TestAsyncStore(t *testing.T) { mem, _ := object.CreateStorage("mem", "", "", "") conf := defaultConf diff --git a/pkg/chunk/utils_unix.go b/pkg/chunk/utils_unix.go index 1846a29b..df3d4650 100644 --- a/pkg/chunk/utils_unix.go +++ b/pkg/chunk/utils_unix.go @@ -42,6 +42,6 @@ func getDiskUsage(path string) (uint64, uint64, uint64, uint64) { func changeMode(dir string, st os.FileInfo, mode os.FileMode) { sst := st.Sys().(*syscall.Stat_t) if os.Getuid() == int(sst.Uid) { - os.Chmod(dir, mode) + _ = os.Chmod(dir, mode) } } diff --git a/pkg/redis/redis.go b/pkg/redis/redis.go index e6fe2cfb..d9ce382a 100644 --- a/pkg/redis/redis.go +++ b/pkg/redis/redis.go @@ -1073,7 +1073,9 @@ func (r *redisMeta) cleanStaleSession(sid int64) { } for _, sinode := range inodes { inode, _ := strconv.Atoi(sinode) - r.deleteInode(Ino(inode)) + if err := r.deleteInode(Ino(inode)); err != nil { + logger.Errorf("Failed to delete inode %d: %s", inode, err) + } } if len(inodes) == 0 { r.rdb.Del(c, r.sessionKey(sid)) @@ -1347,7 +1349,7 @@ func (r *redisMeta) deleteChunks(inode Ino, start, end, maxchunk uint64) { r.rdb.ZAdd(c, delchunks, &redis.Z{Score: float64(now.Unix()), Member: key}) return } - r.txn(func(tx *redis.Tx) error { + _ = r.txn(func(tx *redis.Tx) error { val, err := tx.LRange(c, r.chunkKey(inode, indx+uint32(j)), 0, 1).Result() if err != nil { return err diff --git a/pkg/redis/redis_test.go b/pkg/redis/redis_test.go index fc217bf4..cb63616b 100644 --- a/pkg/redis/redis_test.go +++ b/pkg/redis/redis_test.go @@ -24,6 +24,7 @@ import ( "github.com/juicedata/juicefs/pkg/meta" ) +// nolint:errcheck func TestRedisClient(t *testing.T) { var conf RedisConfig m, err := NewRedisMeta("redis://127.0.0.1:6379/7", &conf) @@ -32,7 +33,7 @@ func TestRedisClient(t *testing.T) { t.Skip() } m.OnMsg(meta.DeleteChunk, func(args ...interface{}) error { return nil }) - m.Init(meta.Format{Name: "test"}) + _ = m.Init(meta.Format{Name: "test"}) ctx := meta.Background var parent, inode meta.Ino var attr = &meta.Attr{} @@ -72,7 +73,9 @@ func TestRedisClient(t *testing.T) { if st := m.Rename(ctx, parent, "f", 1, "f2", &inode, attr); st != 0 { t.Fatalf("rename f %s", st) } - defer m.Unlink(ctx, 1, "f2") + defer func() { + _ = m.Unlink(ctx, 1, "f2") + }() if st := m.Lookup(ctx, 1, "f2", &inode, attr); st != 0 { t.Fatalf("lookup f2: %s", st) } @@ -222,7 +225,7 @@ func TestCompaction(t *testing.T) { t.Logf("redis is not available: %s", err) t.Skip() } - m.Init(meta.Format{Name: "test"}) + _ = m.Init(meta.Format{Name: "test"}) done := make(chan bool, 1) m.OnMsg(meta.CompactChunk, func(args ...interface{}) error { select { @@ -237,7 +240,9 @@ func TestCompaction(t *testing.T) { if st := m.Create(ctx, 1, "f", 0650, 022, &inode, attr); st != 0 { t.Fatalf("create file %s", st) } - defer m.Unlink(ctx, 1, "f") + defer func() { + _ = m.Unlink(ctx, 1, "f") + }() for i := 0; i < 50; i++ { if st := m.Write(ctx, inode, 0, uint32(i*100), meta.Slice{Chunkid: uint64(i) + 1, Size: 100, Len: 100}); st != 0 { t.Fatalf("write %d: %s", i, st) @@ -280,16 +285,14 @@ func TestConcurrentWrite(t *testing.T) { return nil }) _ = m.Init(meta.Format{Name: "test"}) - if err != nil { - t.Fatalf("Failed to initialize meta: %s", err) - } ctx := meta.Background var inode meta.Ino var attr = &meta.Attr{} - m.Unlink(ctx, 1, "f") + _ = m.Unlink(ctx, 1, "f") if st := m.Create(ctx, 1, "f", 0650, 022, &inode, attr); st != 0 { t.Fatalf("create file %s", st) } + // nolint:errcheck defer m.Unlink(ctx, 1, "f") var errno syscall.Errno diff --git a/pkg/vfs/vfs.go b/pkg/vfs/vfs.go index c4e3f430..e556e527 100644 --- a/pkg/vfs/vfs.go +++ b/pkg/vfs/vfs.go @@ -495,10 +495,10 @@ func Release(ctx Context, ino Ino, fh uint64) (err syscall.Errno) { f.writer.Close(ctx) } if locks&1 != 0 { - m.Flock(ctx, ino, owner, syscall.F_UNLCK, false) + _ = m.Flock(ctx, ino, owner, syscall.F_UNLCK, false) } } - m.Close(ctx, ino) + _ = m.Close(ctx, ino) go releaseFileHandle(ino, fh) // after writes it waits for data sync, so do it after everything } return @@ -681,7 +681,7 @@ func Flush(ctx Context, ino Ino, fh uint64, lockOwner uint64) (err syscall.Errno locks := h.locks h.Unlock() if locks&2 != 0 { - m.Setlk(ctx, ino, lockOwner, false, syscall.F_UNLCK, 0, 0x7FFFFFFFFFFFFFFF, 0) + _ = m.Setlk(ctx, ino, lockOwner, false, syscall.F_UNLCK, 0, 0x7FFFFFFFFFFFFFFF, 0) } return } diff --git a/pkg/vfs/vfs_unix.go b/pkg/vfs/vfs_unix.go index e0c3b1f6..cd7e9522 100644 --- a/pkg/vfs/vfs_unix.go +++ b/pkg/vfs/vfs_unix.go @@ -46,7 +46,7 @@ type Statfs struct { func StatFS(ctx Context, ino Ino) (st *Statfs, err int) { var totalspace, availspace, iused, iavail uint64 - m.StatFS(ctx, &totalspace, &availspace, &iused, &iavail) + _ = m.StatFS(ctx, &totalspace, &availspace, &iused, &iavail) var bsize uint64 = 0x10000 blocks := totalspace / bsize bavail := blocks - (totalspace-availspace+bsize-1)/bsize diff --git a/pkg/vfs/writer.go b/pkg/vfs/writer.go index 35d03b99..e6cea2b5 100644 --- a/pkg/vfs/writer.go +++ b/pkg/vfs/writer.go @@ -135,7 +135,11 @@ func (s *sliceWriter) write(ctx meta.Context, off uint32, data []uint8) syscall. go s.flushData() } else if int(s.slen) >= f.w.blockSize { if s.id > 0 { - s.writer.FlushTo(int(s.slen)) + err := s.writer.FlushTo(int(s.slen)) + if err != nil { + logger.Warnf("write: chunk: %d off: %d %s", s.id, off, err) + return syscall.EIO + } } else if int(off) <= f.w.blockSize { go s.prepareID(ctx, false) } -- GitLab