From 7743265113401a3582d850d8c85ca5152eb97624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 16:37:51 +0300 Subject: [PATCH 1/9] Rewrite if-else chains as switches --- internal/common/common_linux.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/common/common_linux.go b/internal/common/common_linux.go index 7349989..ea153de 100644 --- a/internal/common/common_linux.go +++ b/internal/common/common_linux.go @@ -149,16 +149,17 @@ func VirtualizationWithContext(ctx context.Context) (string, string, error) { if PathExists(filename) { contents, err := ReadLines(filename) if err == nil { - if StringsContains(contents, "kvm") { + switch { + case StringsContains(contents, "kvm"): system = "kvm" role = "host" - } else if StringsContains(contents, "vboxdrv") { + case StringsContains(contents, "vboxdrv"): system = "vbox" role = "host" - } else if StringsContains(contents, "vboxguest") { + case StringsContains(contents, "vboxguest"): system = "vbox" role = "guest" - } else if StringsContains(contents, "vmware") { + case StringsContains(contents, "vmware"): system = "vmware" role = "guest" } @@ -224,16 +225,17 @@ func VirtualizationWithContext(ctx context.Context) (string, string, error) { if PathExists(filepath.Join(filename, "self", "cgroup")) { contents, err := ReadLines(filepath.Join(filename, "self", "cgroup")) if err == nil { - if StringsContains(contents, "lxc") { + switch { + case StringsContains(contents, "lxc"): system = "lxc" role = "guest" - } else if StringsContains(contents, "docker") { + case StringsContains(contents, "docker"): system = "docker" role = "guest" - } else if StringsContains(contents, "machine-rkt") { + case StringsContains(contents, "machine-rkt"): system = "rkt" role = "guest" - } else if PathExists("/usr/bin/lxc-version") { + case PathExists("/usr/bin/lxc-version"): system = "lxc" role = "host" } From a9b1ce2deccf781e19b71dce01181a7f60ee1b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 16:40:11 +0300 Subject: [PATCH 2/9] Remove unnecessary conversions --- internal/common/binary.go | 2 +- internal/common/common.go | 2 +- internal/common/common_linux.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/common/binary.go b/internal/common/binary.go index 9b5dc55..bf385fd 100644 --- a/internal/common/binary.go +++ b/internal/common/binary.go @@ -253,7 +253,7 @@ func Write(w io.Writer, order ByteOrder, data interface{}) error { b[0] = *v case uint8: bs = b[:1] - b[0] = byte(v) + b[0] = v case []uint8: bs = v case *int16: diff --git a/internal/common/common.go b/internal/common/common.go index f1e4154..fe01b98 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -165,7 +165,7 @@ func UintToString(orig []uint8) string { size = i break } - ret[i] = byte(o) + ret[i] = o } if size == -1 { size = len(orig) diff --git a/internal/common/common_linux.go b/internal/common/common_linux.go index ea153de..7dc53c4 100644 --- a/internal/common/common_linux.go +++ b/internal/common/common_linux.go @@ -26,8 +26,8 @@ func DoSysctrl(mib string) ([]string, error) { return []string{}, err } v := strings.Replace(string(out), "{ ", "", 1) - v = strings.Replace(string(v), " }", "", 1) - values := strings.Fields(string(v)) + v = strings.Replace(v, " }", "", 1) + values := strings.Fields(v) return values, nil } From 65616500e8e9f946141a8e6d21544666875f09d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 16:41:12 +0300 Subject: [PATCH 3/9] Remove unnecessary empty lines --- internal/common/common_linux.go | 2 -- load/load_linux.go | 1 - load/load_test.go | 1 - 3 files changed, 4 deletions(-) diff --git a/internal/common/common_linux.go b/internal/common/common_linux.go index 7dc53c4..01ae751 100644 --- a/internal/common/common_linux.go +++ b/internal/common/common_linux.go @@ -55,7 +55,6 @@ func NumProcs() (uint64, error) { } func BootTimeWithContext(ctx context.Context) (uint64, error) { - system, role, err := Virtualization() if err != nil { return 0, err @@ -202,7 +201,6 @@ func VirtualizationWithContext(ctx context.Context) (string, string, error) { if PathExists(filepath.Join(filename, "self", "status")) { contents, err := ReadLines(filepath.Join(filename, "self", "status")) if err == nil { - if StringsContains(contents, "s_context:") || StringsContains(contents, "VxID:") { system = "linux-vserver" diff --git a/load/load_linux.go b/load/load_linux.go index 38b4b03..0859397 100644 --- a/load/load_linux.go +++ b/load/load_linux.go @@ -103,7 +103,6 @@ func MiscWithContext(ctx context.Context) (*MiscStat, error) { default: continue } - } procsTotal, err := getProcsTotal() diff --git a/load/load_test.go b/load/load_test.go index 2c809b9..d2739ba 100644 --- a/load/load_test.go +++ b/load/load_test.go @@ -70,7 +70,6 @@ func TestMiscStatString(t *testing.T) { } func BenchmarkLoad(b *testing.B) { - loadAvg := func(t testing.TB) { v, err := Avg() skipIfNotImplementedErr(t, err) From 803cea9d67aafde4509e9384dfb9da26aa483298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 16:44:20 +0300 Subject: [PATCH 4/9] Don't use underscores in variable names --- load/load_linux.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/load/load_linux.go b/load/load_linux.go index 0859397..2a1c5ea 100644 --- a/load/load_linux.go +++ b/load/load_linux.go @@ -31,11 +31,11 @@ func sysinfoAvgWithContext(ctx context.Context) (*AvgStat, error) { return nil, err } - const si_load_shift = 16 + const siLoadShift = 16 return &AvgStat{ - Load1: float64(info.Loads[0]) / float64(1< Date: Wed, 18 Aug 2021 16:47:15 +0300 Subject: [PATCH 5/9] Comment convention fixes --- internal/common/common.go | 18 +++++++++--------- internal/common/common_linux.go | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/common/common.go b/internal/common/common.go index fe01b98..6346288 100644 --- a/internal/common/common.go +++ b/internal/common/common.go @@ -94,7 +94,7 @@ func (i FakeInvoke) CommandWithContext(ctx context.Context, name string, arg ... var ErrNotImplementedError = errors.New("not implemented yet") -// ReadFile reads contents from a file +// ReadFile reads contents from a file. func ReadFile(filename string) (string, error) { content, err := ioutil.ReadFile(filename) @@ -111,7 +111,7 @@ func ReadLines(filename string) ([]string, error) { return ReadLinesOffsetN(filename, 0, -1) } -// ReadLines reads contents from file and splits them by new line. +// ReadLinesOffsetN reads contents from file and splits them by new line. // The offset tells at which line number to start. // The count determines the number of lines to read (starting from offset): // n >= 0: at most n lines @@ -224,31 +224,31 @@ func ReadInts(filename string) ([]int64, error) { return ret, nil } -// Parse Hex to uint32 without error +// HexToUint32 parses Hex to uint32 without error. func HexToUint32(hex string) uint32 { vv, _ := strconv.ParseUint(hex, 16, 32) return uint32(vv) } -// Parse to int32 without error +// mustParseInt32 parses to int32 without error. func mustParseInt32(val string) int32 { vv, _ := strconv.ParseInt(val, 10, 32) return int32(vv) } -// Parse to uint64 without error +// mustParseUint64 parses to uint64 without error. func mustParseUint64(val string) uint64 { vv, _ := strconv.ParseInt(val, 10, 64) return uint64(vv) } -// Parse to Float64 without error +// mustParseFloat64 parses to Float64 without error. func mustParseFloat64(val string) float64 { vv, _ := strconv.ParseFloat(val, 64) return vv } -// StringsHas checks the target string slice contains src or not +// StringsHas checks the target string slice contains src or not. func StringsHas(target []string, src string) bool { for _, t := range target { if strings.TrimSpace(t) == src { @@ -258,7 +258,7 @@ func StringsHas(target []string, src string) bool { return false } -// StringsContains checks the src in any string of the target string slice +// StringsContains checks the src in any string of the target string slice. func StringsContains(target []string, src string) bool { for _, t := range target { if strings.Contains(t, src) { @@ -308,7 +308,7 @@ func PathExists(filename string) bool { return false } -//GetEnv retrieves the environment variable key. If it does not exist it returns the default. +// GetEnv retrieves the environment variable key. If it does not exist it returns the default. func GetEnv(key string, dfault string, combineWith ...string) string { value := os.Getenv(key) if value == "" { diff --git a/internal/common/common_linux.go b/internal/common/common_linux.go index 01ae751..c4be7f1 100644 --- a/internal/common/common_linux.go +++ b/internal/common/common_linux.go @@ -110,7 +110,7 @@ func Virtualization() (string, string, error) { return VirtualizationWithContext(context.Background()) } -// required variables for concurrency safe virtualization caching +// required variables for concurrency safe virtualization caching. var ( cachedVirtMap map[string]string cachedVirtMutex sync.RWMutex @@ -281,7 +281,7 @@ func GetOSRelease() (platform string, version string, err error) { return platform, version, nil } -// Remove quotes of the source string +// trimQuotes removes quotes in the source string. func trimQuotes(s string) string { if len(s) >= 2 { if s[0] == '"' && s[len(s)-1] == '"' { From ce9d35436eaeec09dca5d2dfc2d8dd2278b9deee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 16:50:02 +0300 Subject: [PATCH 6/9] Merge variable declaration with assignment --- internal/common/common_unix.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/common/common_unix.go b/internal/common/common_unix.go index 9e393bc..6052028 100644 --- a/internal/common/common_unix.go +++ b/internal/common/common_unix.go @@ -41,8 +41,7 @@ func CallLsofWithContext(ctx context.Context, invoke Invoker, pid int32, args .. } func CallPgrepWithContext(ctx context.Context, invoke Invoker, pid int32) ([]int32, error) { - var cmd []string - cmd = []string{"-P", strconv.Itoa(int(pid))} + cmd := []string{"-P", strconv.Itoa(int(pid))} pgrep, err := exec.LookPath("pgrep") if err != nil { return []int32{}, err From 633e77013fa384db3426a47cea0f3a7da0e11926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 16:51:44 +0300 Subject: [PATCH 7/9] Use short if --- internal/common/common_test.go | 9 +++------ load/load_linux.go | 3 +-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/common/common_test.go b/internal/common/common_test.go index b0e051c..d9922af 100644 --- a/internal/common/common_test.go +++ b/internal/common/common_test.go @@ -32,8 +32,7 @@ func TestReadLinesOffsetN(t *testing.T) { func TestIntToString(t *testing.T) { src := []int8{65, 66, 67} - dst := IntToString(src) - if dst != "ABC" { + if dst := IntToString(src); dst != "ABC" { t.Error("could not convert") } } @@ -58,8 +57,7 @@ func TestHexToUint32(t *testing.T) { } func TestMustParseInt32(t *testing.T) { - ret := mustParseInt32("11111") - if ret != int32(11111) { + if ret := mustParseInt32("11111"); ret != int32(11111) { t.Error("could not parse") } } @@ -102,8 +100,7 @@ func TestHostEtc(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("windows doesn't have etc") } - p := HostEtc("mtab") - if p != "/etc/mtab" { + if p := HostEtc("mtab"); p != "/etc/mtab" { t.Errorf("invalid HostEtc, %s", p) } } diff --git a/load/load_linux.go b/load/load_linux.go index 2a1c5ea..08699f4 100644 --- a/load/load_linux.go +++ b/load/load_linux.go @@ -26,8 +26,7 @@ func AvgWithContext(ctx context.Context) (*AvgStat, error) { func sysinfoAvgWithContext(ctx context.Context) (*AvgStat, error) { var info syscall.Sysinfo_t - err := syscall.Sysinfo(&info) - if err != nil { + if err := syscall.Sysinfo(&info); err != nil { return nil, err } From fb0c32226034666b337e2282f0d14d26e1533845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 17:04:01 +0300 Subject: [PATCH 8/9] Check error identity with errors.Is --- internal/common/sleep_test.go | 3 ++- load/load_test.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/common/sleep_test.go b/internal/common/sleep_test.go index 5ecb6ea..bba4661 100644 --- a/internal/common/sleep_test.go +++ b/internal/common/sleep_test.go @@ -2,6 +2,7 @@ package common_test import ( "context" + "errors" "testing" "time" @@ -13,7 +14,7 @@ func TestSleep(test *testing.T) { var t = func(name string, ctx context.Context, expected error) { test.Run(name, func(test *testing.T) { var err = common.Sleep(ctx, dt) - if err != expected { + if !errors.Is(err, expected) { test.Errorf("expected %v, got %v", expected, err) } }) diff --git a/load/load_test.go b/load/load_test.go index d2739ba..4c9483b 100644 --- a/load/load_test.go +++ b/load/load_test.go @@ -1,6 +1,7 @@ package load import ( + "errors" "fmt" "testing" @@ -8,7 +9,7 @@ import ( ) func skipIfNotImplementedErr(t testing.TB, err error) { - if err == common.ErrNotImplementedError { + if errors.Is(err, common.ErrNotImplementedError) { t.Skip("not implemented") } } From a21240a319c145928e7f8240446360583eb3ad4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 18 Aug 2021 17:08:30 +0300 Subject: [PATCH 9/9] Simplify some if blocks --- internal/common/common_linux.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/internal/common/common_linux.go b/internal/common/common_linux.go index c4be7f1..ca01c75 100644 --- a/internal/common/common_linux.go +++ b/internal/common/common_linux.go @@ -75,6 +75,18 @@ func BootTimeWithContext(ctx context.Context) (uint64, error) { return 0, err } + if statFile == "uptime" { + if len(lines) != 1 { + return 0, fmt.Errorf("wrong uptime format") + } + f := strings.Fields(lines[0]) + b, err := strconv.ParseFloat(f[0], 64) + if err != nil { + return 0, err + } + t := uint64(time.Now().Unix()) - uint64(b) + return t, nil + } if statFile == "stat" { for _, line := range lines { if strings.HasPrefix(line, "btime") { @@ -90,17 +102,6 @@ func BootTimeWithContext(ctx context.Context) (uint64, error) { return t, nil } } - } else if statFile == "uptime" { - if len(lines) != 1 { - return 0, fmt.Errorf("wrong uptime format") - } - f := strings.Fields(lines[0]) - b, err := strconv.ParseFloat(f[0], 64) - if err != nil { - return 0, err - } - t := uint64(time.Now().Unix()) - uint64(b) - return t, nil } return 0, fmt.Errorf("could not find btime") @@ -136,10 +137,8 @@ func VirtualizationWithContext(ctx context.Context) (string, string, error) { if PathExists(filepath.Join(filename, "capabilities")) { contents, err := ReadLines(filepath.Join(filename, "capabilities")) - if err == nil { - if StringsContains(contents, "control_d") { - role = "host" - } + if err == nil && StringsContains(contents, "control_d") { + role = "host" } } }