Removed use of ioutil.ReadDir as it performs a stat syscall on each file which is not necerssary.

This commit is contained in:
Karim Kanso
2020-03-11 09:17:55 +00:00
parent 4f3946e673
commit b331c4c45e
3 changed files with 157 additions and 104 deletions

View File

@@ -1,17 +1,12 @@
package psscanner package psscanner
import ( import (
"errors"
"fmt" "fmt"
"io/ioutil" "io"
"os" "os"
"strconv" "strconv"
) )
var procDirReader = func() ([]os.FileInfo, error) {
return ioutil.ReadDir("/proc")
}
type procList map[int]struct{} type procList map[int]struct{}
type pidProcessor interface { type pidProcessor interface {
@@ -37,15 +32,21 @@ func (pl procList) refresh(p pidProcessor) error {
} }
func getPIDs() ([]int, error) { func getPIDs() ([]int, error) {
proc, err := procDirReader() f, err := dirOpen("/proc")
if err != nil { if err != nil {
return nil, fmt.Errorf("opening proc dir: %v", err) return nil, fmt.Errorf("opening proc dir: %v", err)
} }
defer f.Close()
names, err := f.Readdirnames(-1)
if err != nil {
return nil, fmt.Errorf("reading proc dir: %v", err)
}
pids := make([]int, 0) pids := make([]int, 0)
for _, f := range proc { for _, f := range names {
pid, err := file2Pid(f) pid, err := strconv.Atoi(f)
if err != nil { if err != nil || pid <= 0 {
continue continue
} }
pids = append(pids, pid) pids = append(pids, pid)
@@ -53,17 +54,11 @@ func getPIDs() ([]int, error) {
return pids, nil return pids, nil
} }
var errNotAPid = errors.New("not a pid") type readDirNamesCloser interface {
Readdirnames(n int) (names []string, err error)
func file2Pid(f os.FileInfo) (int, error) { io.Closer
if !f.IsDir() { }
return -1, errNotAPid
} var dirOpen func(string) (readDirNamesCloser, error) = func(s string) (readDirNamesCloser, error) {
return os.Open(s)
pid, err := strconv.Atoi(f.Name())
if err != nil || pid <= 0 {
return -1, errNotAPid
}
return pid, nil
} }

View File

@@ -3,90 +3,129 @@ package psscanner
import ( import (
"errors" "errors"
"fmt" "fmt"
"os"
"reflect" "reflect"
"strings" "strings"
"testing" "testing"
"time"
) )
// GetPIDs // GetPIDs
func TestGetPIDs(t *testing.T) { func TestGetPIDs(t *testing.T) {
tests := []struct { tests := []struct {
proc []os.FileInfo name string
procErr error proc []string
pids []int procErrOpen error
err string procErrRead error
pids []int
err string
}{ }{
{proc: []os.FileInfo{newMockDir("42"), newMockDir("somedir")}, procErr: nil, pids: []int{42}, err: ""}, // reads numbers and ignores everything else {
{proc: []os.FileInfo{newMockDir("42"), newMockFile("13")}, procErr: nil, pids: []int{42}, err: ""}, // reads directories and ignores files name: "numbers-only",
{proc: []os.FileInfo{newMockDir("0"), newMockDir("-1")}, procErr: nil, pids: []int{}, err: ""}, // ignores 0 and negative numbers proc: []string{"42", "somedir"},
{proc: []os.FileInfo{}, procErr: nil, pids: []int{}, err: ""}, // can handle empty procfs procErrOpen: nil,
{proc: []os.FileInfo{}, procErr: errors.New("file-system-error"), pids: nil, err: "opening proc dir: file-system-error"}, // returns errors procErrRead: nil,
pids: []int{42},
err: "",
},
{
name: "multiple-entries",
proc: []string{"42", "13"},
procErrOpen: nil,
procErrRead: nil,
pids: []int{42, 13},
err: "",
},
{
name: "ignores-lte-0",
proc: []string{"0", "-1"},
procErrOpen: nil,
procErrRead: nil,
pids: []int{},
err: "",
},
{
name: "empty-procfs",
proc: []string{},
procErrOpen: nil,
procErrRead: nil,
pids: []int{},
err: "",
},
{
name: "handle-open-error",
proc: []string{},
procErrOpen: errors.New("file-system-error"),
procErrRead: nil,
pids: nil,
err: "opening proc dir: file-system-error",
},
{
name: "handle-read-error",
proc: []string{},
procErrOpen: nil,
procErrRead: errors.New("file-system-error"),
pids: nil,
err: "reading proc dir: file-system-error",
},
} }
for _, tt := range tests { for _, tt := range tests {
restore := mockProcDirReader(tt.proc, tt.procErr) t.Run(tt.name, func(t *testing.T) {
pids, err := getPIDs() defer mockDir("/proc", tt.proc, tt.procErrRead, tt.procErrOpen, t)()
if !reflect.DeepEqual(pids, tt.pids) { pids, err := getPIDs()
t.Errorf("Wrong pids returned: got %v but want %v", pids, tt.pids) if !reflect.DeepEqual(pids, tt.pids) {
} t.Errorf("Wrong pids returned: got %v but want %v", pids, tt.pids)
if (err != nil || tt.err != "") && fmt.Sprintf("%v", err) != tt.err { }
t.Errorf("Wrong error returned: got %v but want %s", err, tt.err) if (err != nil || tt.err != "") && fmt.Sprintf("%v", err) != tt.err {
} t.Errorf("Wrong error returned: got %v but want %s", err, tt.err)
restore() }
})
} }
} }
func mockProcDirReader(proc []os.FileInfo, err error) (restore func()) { type MockDir struct {
oldFunc := procDirReader names []string
procDirReader = func() ([]os.FileInfo, error) { err error
return proc, err }
func (f *MockDir) Close() error {
return nil
}
func min(a, b int) int {
if a > b {
return b
}
return a
}
func (f *MockDir) Readdirnames(n int) (names []string, err error) {
if n < 0 {
return f.names, f.err
}
return f.names[:min(n, len(f.names))], f.err
}
// Hook/chain a mocked file into the "open" variable
func mockDir(name string, names []string, errRead error, errOpen error, t *testing.T) func() {
oldopen := dirOpen
dirOpen = func(n string) (readDirNamesCloser, error) {
if name == n {
if testing.Verbose() {
t.Logf("opening mocked dir: %s", n)
}
return &MockDir{
names: names,
err: errRead,
}, errOpen
}
return oldopen(n)
} }
return func() { return func() {
procDirReader = oldFunc dirOpen = oldopen
} }
} }
func newMockDir(name string) *mockFileInfo {
return &mockFileInfo{
name: name,
isDir: true,
}
}
func newMockFile(name string) *mockFileInfo {
return &mockFileInfo{
name: name,
isDir: false,
}
}
type mockFileInfo struct {
name string
isDir bool
}
func (f *mockFileInfo) Name() string {
return f.name
}
func (f *mockFileInfo) Size() int64 {
return 0
}
func (f *mockFileInfo) Mode() os.FileMode {
return 0
}
func (f *mockFileInfo) ModTime() time.Time {
return time.Now()
}
func (f *mockFileInfo) IsDir() bool {
return f.isDir
}
func (f *mockFileInfo) Sys() interface{} {
return nil
}
type mockPidProcessor struct { type mockPidProcessor struct {
t *testing.T t *testing.T
pids []int pids []int
@@ -124,7 +163,7 @@ func TestRefresh(t *testing.T) {
pidsProcessed: []int{3, 2}, pidsProcessed: []int{3, 2},
}, },
{ {
name: "nothing new", name: "nothing-new",
pl: procList{1: unit, 2: unit, 3: unit}, pl: procList{1: unit, 2: unit, 3: unit},
newPids: []int{1, 2, 3}, newPids: []int{1, 2, 3},
plAfter: procList{1: unit, 2: unit, 3: unit}, plAfter: procList{1: unit, 2: unit, 3: unit},
@@ -134,7 +173,7 @@ func TestRefresh(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
defer mockPidList(tt.newPids)() defer mockPidList(tt.newPids, t)()
m := &mockPidProcessor{t, []int{}} m := &mockPidProcessor{t, []int{}}
tt.pl.refresh(m) tt.pl.refresh(m)
@@ -152,24 +191,43 @@ func TestRefresh(t *testing.T) {
// separate test for failing, only one case where getPids fails // separate test for failing, only one case where getPids fails
func TestRefreshFail(t *testing.T) { func TestRefreshFail(t *testing.T) {
e := errors.New("file-system-error") e := errors.New("file-system-error")
defer mockProcDirReader([]os.FileInfo{}, e)() for _, tt := range []struct {
m := &mockPidProcessor{t, []int{}} name string
pl := procList{1: unit} errRead error
err := pl.refresh(m) errOpen error
if err == nil { }{
t.Errorf("Expected an error") {
} else { name: "open-dir-fail",
if strings.Index(err.Error(), e.Error()) == -1 { errRead: nil,
t.Errorf("Unexpected error: %v", err) errOpen: e,
} },
{
name: "read-dir-fail",
errRead: e,
errOpen: nil,
},
} {
t.Run(tt.name, func(t *testing.T) {
defer mockDir("/proc", []string{}, tt.errRead, tt.errOpen, t)()
m := &mockPidProcessor{t, []int{}}
pl := procList{1: unit}
err := pl.refresh(m)
if err == nil {
t.Errorf("Expected an error")
} else {
if strings.Index(err.Error(), e.Error()) == -1 {
t.Errorf("Unexpected error: %v", err)
}
}
})
} }
} }
func mockPidList(pids []int) func() { func mockPidList(pids []int, t *testing.T) func() {
dirs := make([]os.FileInfo, 0) dirs := make([]string, 0)
for _, pid := range pids { for _, pid := range pids {
dirs = append(dirs, newMockDir(fmt.Sprintf("%d", pid))) dirs = append(dirs, fmt.Sprintf("%d", pid))
} }
restore := mockProcDirReader(dirs, nil) return mockDir("/proc", dirs, nil, nil, t)
return restore
} }

View File

@@ -31,7 +31,7 @@ func TestRun(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
defer mockPidList(tt.pids)() defer mockPidList(tt.pids, t)()
for _, pid := range tt.pids { for _, pid := range tt.pids {
defer mockPidCmdLine(pid, []byte("the-command"), nil, nil, t)() defer mockPidCmdLine(pid, []byte("the-command"), nil, nil, t)()
defer mockPidStatus(pid, []byte{}, nil, nil, t)() // don't mock read value since it's not worth it defer mockPidStatus(pid, []byte{}, nil, nil, t)() // don't mock read value since it's not worth it