Fix all errors reported by golangci-lint and set up pre-commit hook (#43)

Co-authored-by: NDavies Liu <davies@juicedata.io>

.golangci.yml

+run:
+  timeout: 5m
+  skip-dirs:
+    - fstests

.pre-commit-config.yaml

+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

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
 

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)

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

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)
 	}

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()
 }
 

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" {

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}
 }

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

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)
 	}
 }

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

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

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
 }

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

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)
 		}