refactor times_linux.go and add tests

djherbis · Oct 3, 2023 · 9460724 · 9460724
1 parent 03d4552
commit 9460724
Show file tree

Hide file tree

Showing 5 changed files with 229 additions and 58 deletions.
diff --git a/linux.cover.dockerfile b/linux.cover.dockerfile
@@ -0,0 +1,6 @@
+FROM golang:1.17
+
+WORKDIR /go/src/github.com/djherbis/times
+COPY . .
+
+RUN GO111MODULE=auto go test -covermode=count -coverprofile=profile.cov
diff --git a/linux.cover.sh b/linux.cover.sh
@@ -0,0 +1,7 @@
+#!/bin/bash
+set -e
+
+docker build -f linux.cover.dockerfile -t linux.cover.djherbis.times .
+docker create --name linux.cover.djherbis.times linux.cover.djherbis.times
+docker cp linux.cover.djherbis.times:/go/src/github.com/djherbis/times/profile.cov .
+docker rm -v linux.cover.djherbis.times
diff --git a/times_linux.go b/times_linux.go
@@ -37,85 +37,113 @@ type timespecBtime struct {
 	btime
 }
 
-var supportsStatx int32 = 1
+var (
+	supportsStatx int32 = 1
+	statxFunc           = unix.Statx
+)
+
+func isStatXSupported() bool {
+	return atomic.LoadInt32(&supportsStatx) == 1
+}
+
+func isStatXUnsupported(err error) bool {
+	// linux 4.10 and earlier does not support Statx syscall
+	if err != nil && errors.Is(err, unix.ENOSYS) {
+		atomic.StoreInt32(&supportsStatx, 0)
+		return true
+	}
+	return false
+}
 
 // Stat returns the Timespec for the given filename.
 func Stat(name string) (Timespec, error) {
-	if atomic.LoadInt32(&supportsStatx) == 1 {
-		var statx unix.Statx_t
-
-		//https://man7.org/linux/man-pages/man2/statx.2.html
-		err := unix.Statx(unix.AT_FDCWD, name, unix.AT_STATX_SYNC_AS_STAT, unix.STATX_ATIME|unix.STATX_MTIME|unix.STATX_CTIME|unix.STATX_BTIME, &statx)
-		if err != nil {
-			//linux 4.10 and earlier does not support Statx syscall
-			if errors.Is(err, unix.ENOSYS) {
-				atomic.StoreInt32(&supportsStatx, 0)
-				return stat(name, os.Stat)
-			}
+	if isStatXSupported() {
+		ts, err := statX(name)
+		if err == nil {
+			return ts, nil
+		}
+		if !isStatXUnsupported(err) {
 			return nil, err
 		}
-		return extractTimes(&statx), nil
+		// Fallback.
 	}
-
 	return stat(name, os.Stat)
 }
 
+func statX(name string) (Timespec, error) {
+	// https://man7.org/linux/man-pages/man2/statx.2.html
+	var statx unix.Statx_t
+	err := statxFunc(unix.AT_FDCWD, name, unix.AT_STATX_SYNC_AS_STAT, unix.STATX_ATIME|unix.STATX_MTIME|unix.STATX_CTIME|unix.STATX_BTIME, &statx)
+	if err != nil {
+		return nil, err
+	}
+	return extractTimes(&statx), nil
+}
+
 // Lstat returns the Timespec for the given filename, and does not follow Symlinks.
 func Lstat(name string) (Timespec, error) {
-	if atomic.LoadInt32(&supportsStatx) == 1 {
-		var statX unix.Statx_t
-		//https://man7.org/linux/man-pages/man2/statx.2.html
-
-		err := unix.Statx(unix.AT_FDCWD, name, unix.AT_STATX_SYNC_AS_STAT|unix.AT_SYMLINK_NOFOLLOW, unix.STATX_ATIME|unix.STATX_MTIME|unix.STATX_CTIME|unix.STATX_BTIME, &statX)
-		if err != nil {
-			//linux 4.10 and earlier does not support Statx syscall
-			if errors.Is(err, unix.ENOSYS) {
-				atomic.StoreInt32(&supportsStatx, 0)
-				return stat(name, os.Lstat)
-			}
+	if isStatXSupported() {
+		ts, err := lstatx(name)
+		if err == nil {
+			return ts, nil
+		}
+		if !isStatXUnsupported(err) {
 			return nil, err
 		}
-		return extractTimes(&statX), nil
+		// Fallback.
 	}
-
 	return stat(name, os.Lstat)
 }
 
+func lstatx(name string) (Timespec, error) {
+	// https://man7.org/linux/man-pages/man2/statx.2.html
+	var statX unix.Statx_t
+	err := statxFunc(unix.AT_FDCWD, name, unix.AT_STATX_SYNC_AS_STAT|unix.AT_SYMLINK_NOFOLLOW, unix.STATX_ATIME|unix.STATX_MTIME|unix.STATX_CTIME|unix.STATX_BTIME, &statX)
+	if err != nil {
+		return nil, err
+	}
+	return extractTimes(&statX), nil
+}
+
+func statXFile(file *os.File) (Timespec, error) {
+	sc, err := file.SyscallConn()
+	if err != nil {
+		return nil, err
+	}
+
+	var statx unix.Statx_t
+	var statxErr error
+	err = sc.Control(func(fd uintptr) {
+		// https://man7.org/linux/man-pages/man2/statx.2.html
+		statxErr = statxFunc(int(fd), "", unix.AT_EMPTY_PATH|unix.AT_STATX_SYNC_AS_STAT, unix.STATX_ATIME|unix.STATX_MTIME|unix.STATX_CTIME|unix.STATX_BTIME, &statx)
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	if statxErr != nil {
+		return nil, statxErr
+	}
+
+	return extractTimes(&statx), nil
+}
+
 // StatFile returns the Timespec for the given *os.File.
 func StatFile(file *os.File) (Timespec, error) {
-	if atomic.LoadInt32(&supportsStatx) == 1 {
-		var statx unix.Statx_t
-
-		sc, err := file.SyscallConn()
-		if err != nil {
-			return nil, err
+	if isStatXSupported() {
+		ts, err := statXFile(file)
+		if err == nil {
+			return ts, nil
 		}
-
-		var statxErr error
-		err = sc.Control(func(fd uintptr) {
-			statxErr = unix.Statx(int(fd), "", unix.AT_EMPTY_PATH|unix.AT_STATX_SYNC_AS_STAT, unix.STATX_ATIME|unix.STATX_MTIME|unix.STATX_CTIME|unix.STATX_BTIME, &statx)
-		})
-		if err != nil {
+		if !isStatXUnsupported(err) {
 			return nil, err
 		}
-
-		//https://man7.org/linux/man-pages/man2/statx.2.html
-		if statxErr != nil {
-			//linux 4.10 and earlier does not support Statx syscall
-			if errors.Is(statxErr, unix.ENOSYS) {
-				atomic.StoreInt32(&supportsStatx, 0)
-				fi, err := file.Stat()
-				if err != nil {
-					return nil, err
-				}
-				return getTimespec(fi), nil
-			}
-			return nil, statxErr
-		}
-
-		return extractTimes(&statx), nil
+		// Fallback.
 	}
+	return statFile(file)
+}
 
+func statFile(file *os.File) (Timespec, error) {
 	fi, err := file.Stat()
 	if err != nil {
 		return nil, err

diff --git a/times_linux_test.go b/times_linux_test.go
@@ -0,0 +1,130 @@
+package times
+
+import (
+	"errors"
+	"os"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"golang.org/x/sys/unix"
+)
+
+func timeToStatx(t time.Time) unix.StatxTimestamp {
+	nsec := time.Duration(t.UnixNano()) * time.Nanosecond
+	nsec -= time.Duration(t.Unix()) * time.Second
+	return unix.StatxTimestamp{Sec: t.Unix(), Nsec: uint32(nsec)}
+}
+
+func statxT(t time.Time, hasBtime bool) *unix.Statx_t {
+	var statx unix.Statx_t
+
+	statxt := timeToStatx(t)
+
+	statx.Atime = statxt
+	statx.Mtime = statxt
+	statx.Ctime = statxt
+
+	if hasBtime {
+		statx.Mask = unix.STATX_BTIME
+		statx.Btime = statxt
+	}
+
+	return &statx
+}
+
+type statxFuncTyp func(dirfd int, path string, flags int, mask int, stat *unix.Statx_t) (err error)
+
+func unsupportedStatx(dirfd int, path string, flags int, mask int, stat *unix.Statx_t) (err error) {
+	return unix.ENOSYS
+}
+
+var badStatxErr = errors.New("bad")
+
+func badStatx(dirfd int, path string, flags int, mask int, stat *unix.Statx_t) (err error) {
+	return badStatxErr
+}
+
+func fakeSupportedStatx(ts *unix.Statx_t) statxFuncTyp {
+	return func(dirfd int, path string, flags int, mask int, stat *unix.Statx_t) (err error) {
+		*stat = *ts
+		return nil
+	}
+}
+
+func setStatx(fn statxFuncTyp) func() {
+	atomic.StoreInt32(&supportsStatx, 1)
+	restoreStatx := statxFunc
+	statxFunc = fn
+	return func() { statxFunc = restoreStatx }
+}
+
+func TestStatx(t *testing.T) {
+	tests := []struct {
+		name    string
+		statx   statxFuncTyp
+		wantErr error
+	}{
+		{name: "unsupported", statx: unsupportedStatx},
+		{name: "fake supported with btime", statx: fakeSupportedStatx(statxT(time.Now(), true))},
+		{name: "fake supported without btime", statx: fakeSupportedStatx(statxT(time.Now(), false))},
+		{name: "bad stat", statx: badStatx, wantErr: badStatxErr},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Run("stat", func(t *testing.T) {
+				restore := setStatx(test.statx)
+				defer restore()
+
+				fileAndDirTest(t, func(name string) {
+					ts, err := Stat(name)
+					if err != nil {
+						if err == test.wantErr {
+							return
+						}
+						t.Fatal(err.Error())
+					}
+					timespecTest(ts, newInterval(time.Now(), time.Second), t)
+				})
+			})
+
+			t.Run("statFile", func(t *testing.T) {
+				restore := setStatx(test.statx)
+				defer restore()
+
+				fileAndDirTest(t, func(name string) {
+					fi, err := os.Open(name)
+					if err != nil {
+						t.Fatal(err.Error())
+					}
+					defer fi.Close()
+
+					ts, err := StatFile(fi)
+					if err != nil {
+						if err == test.wantErr {
+							return
+						}
+						t.Fatal(err.Error())
+					}
+					timespecTest(ts, newInterval(time.Now(), time.Second), t)
+				})
+			})
+
+			t.Run("lstat", func(t *testing.T) {
+				restore := setStatx(test.statx)
+				defer restore()
+
+				fileAndDirTest(t, func(name string) {
+					ts, err := Lstat(name)
+					if err != nil {
+						if err == test.wantErr {
+							return
+						}
+						t.Fatal(err.Error())
+					}
+					timespecTest(ts, newInterval(time.Now(), time.Second), t)
+				})
+			})
+		})
+	}
+}
diff --git a/times_test.go b/times_test.go
@@ -66,19 +66,19 @@ func testStatSymlink(sf tsFunc, expectTime time.Time, t *testing.T) {
 
 		symname := filepath.Join(filepath.Dir(name), "sym-"+filepath.Base(name))
 		if err := os.Symlink(name, symname); err != nil {
-			t.Error(err.Error())
+			t.Fatal(err.Error())
 		}
 		defer os.Remove(symname)
 
 		// modify the realFileTime so symlink and real file see diff values.
 		realFileTime := start.Add(offsetTime)
 		if err := os.Chtimes(name, realFileTime, realFileTime); err != nil {
-			t.Error(err.Error())
+			t.Fatal(err.Error())
 		}
 
 		ts, err := sf(symname)
 		if err != nil {
-			t.Error(err.Error())
+			t.Fatal(err.Error())
 		}
 		timespecTest(ts, newInterval(expectTime, time.Second), t, Timespec.AccessTime, Timespec.ModTime)
 	})