Merge pull request mitchellh#6 from keybase/taruti/cgo-only-on-osx

Separate cgo into darwin specific package

darwincgo/empty.go

@@ -0,0 +1 @@
+package darwincgo

darwincgo/process_darwin.go

@@ -0,0 +1,84 @@
+// +build darwin
+
+package darwincgo
+
+/*
+#include <stdio.h>
+#include <errno.h>
+#include <libproc.h>
+extern int darwinProcesses();
+extern void darwinProcessPaths();
+*/
+import "C"
+
+import (
+	"path/filepath"
+	"sync"
+)
+
+// This lock is what verifies that C calling back into Go is only
+// modifying data once at a time.
+var darwinLock sync.Mutex
+var darwinProcsByPID map[int]*DarwinProcess
+
+// DarwinProcess is process definition for OS X
+type DarwinProcess struct {
+	pid  int
+	ppid int
+	path string
+}
+
+// Pid returns process id
+func (p *DarwinProcess) Pid() int {
+	return p.pid
+}
+
+// PPid returns parent process id
+func (p *DarwinProcess) PPid() int {
+	return p.ppid
+}
+
+// Executable returns process executable name
+func (p *DarwinProcess) Executable() string {
+	path, _ := p.Path()
+	return filepath.Base(path)
+}
+
+// Path returns path to process executable
+func (p *DarwinProcess) Path() (string, error) {
+	return p.path, nil
+}
+
+//export goDarwinAppendProc
+func goDarwinAppendProc(pid C.pid_t, ppid C.pid_t, comm *C.char) {
+	proc := &DarwinProcess{
+		pid:  int(pid),
+		ppid: int(ppid),
+	}
+	darwinProcsByPID[proc.pid] = proc
+}
+
+//export goDarwinSetPath
+func goDarwinSetPath(pid C.pid_t, comm *C.char) {
+	if proc, ok := darwinProcsByPID[int(pid)]; ok && proc != nil {
+		proc.path = C.GoString(comm)
+	}
+}
+
+// ProcessMap returns a map of processes for the main library package.
+func ProcessMap() (map[int]*DarwinProcess, error) {
+	darwinLock.Lock()
+	defer darwinLock.Unlock()
+	darwinProcsByPID = make(map[int]*DarwinProcess)
+
+	// To ignore deadcode warnings for exported functions
+	_ = goDarwinAppendProc
+	_ = goDarwinSetPath
+
+	// TODO: Investigate why darwinProcesses returns error even if process list
+	// succeeds
+	C.darwinProcesses()
+	C.darwinProcessPaths()
+
+	return darwinProcsByPID, nil
+}

process_darwin.go

@@ -2,105 +2,30 @@
 
 package ps
 
-/*
-#include <stdio.h>
-#include <errno.h>
-#include <libproc.h>
-extern int darwinProcesses();
-extern void darwinProcessPaths();
-*/
-import "C"
-
 import (
-	"fmt"
-	"path/filepath"
-	"sync"
+	"github.com/keybase/go-ps/darwincgo"
 )
 
-// This lock is what verifies that C calling back into Go is only
-// modifying data once at a time.
-var darwinLock sync.Mutex
-var darwinProcs []Process
-var darwinProcsByPID map[int]*DarwinProcess
-
-// DarwinProcess is process definition for OS X
-type DarwinProcess struct {
-	pid  int
-	ppid int
-	path string
-}
-
-// Pid returns process id
-func (p *DarwinProcess) Pid() int {
-	return p.pid
-}
-
-// PPid returns parent process id
-func (p *DarwinProcess) PPid() int {
-	return p.ppid
-}
-
-// Executable returns process executable name
-func (p *DarwinProcess) Executable() string {
-	path, _ := p.Path()
-	return filepath.Base(path)
-}
-
-// Path returns path to process executable
-func (p *DarwinProcess) Path() (string, error) {
-	return p.path, nil
-}
-
-//export goDarwinAppendProc
-func goDarwinAppendProc(pid C.pid_t, ppid C.pid_t, comm *C.char) {
-	proc := &DarwinProcess{
-		pid:  int(pid),
-		ppid: int(ppid),
-	}
-	darwinProcs = append(darwinProcs, proc)
-	darwinProcsByPID[proc.pid] = proc
-}
-
-//export goDarwinSetPath
-func goDarwinSetPath(pid C.pid_t, comm *C.char) {
-	if proc, ok := darwinProcsByPID[int(pid)]; ok && proc != nil {
-		proc.path = C.GoString(comm)
-	}
-}
-
 func findProcess(pid int) (Process, error) {
-	return findProcessWithFn(processes, pid)
-}
-
-func findProcessWithFn(processesFn processesFn, pid int) (Process, error) {
-	ps, err := processesFn()
+	m, err := darwincgo.ProcessMap()
 	if err != nil {
-		return nil, fmt.Errorf("Error listing processes: %s", err)
+		return nil, err
 	}
-
-	for _, p := range ps {
-		if p.Pid() == pid {
-			return p, nil
-		}
+	p := m[pid]
+	if p == nil {
+		return nil, nil
 	}
-
-	return nil, nil
+	return p, nil
 }
 
 func processes() ([]Process, error) {
-	darwinLock.Lock()
-	defer darwinLock.Unlock()
-	darwinProcs = make([]Process, 0, 50)
-	darwinProcsByPID = make(map[int]*DarwinProcess)
-
-	// To ignore deadcode warnings for exported functions
-	_ = goDarwinAppendProc
-	_ = goDarwinSetPath
-
-	// TODO: Investigate why darwinProcesses returns error even if process list
-	// succeeds
-	C.darwinProcesses()
-	C.darwinProcessPaths()
-
-	return darwinProcs, nil
+	m, err := darwincgo.ProcessMap()
+	if err != nil {
+		return nil, err
+	}
+	ps := make([]Process, 0, len(m))
+	for _, dp := range m {
+		ps = append(ps, dp)
+	}
+	return ps, nil
 }

process_darwin_test.go

@@ -3,7 +3,6 @@
 package ps
 
 import (
-	"fmt"
 	"os"
 	"testing"
 
@@ -21,12 +20,9 @@ func TestProcessesDarwin(t *testing.T) {
 }
 
 func TestProcessesDarwinError(t *testing.T) {
-	errFn := func() ([]Process, error) {
-		return nil, fmt.Errorf("oops")
-	}
-	proc, err := findProcessWithFn(errFn, os.Getpid())
+	proc, err := findProcess(-1)
 	assert.Nil(t, proc)
-	assert.EqualError(t, err, "Error listing processes: oops")
+	assert.Nil(t, err)
 }
 
 func TestProcessExecRemoved(t *testing.T) {

process_test.go

@@ -1,7 +1,9 @@
 package ps
 
 import (
+	"errors"
 	"os"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -21,6 +23,7 @@ func testFindProcess(t *testing.T, name string) Process {
 		require.NoError(t, err)
 		t.Logf("Path: %s", path)
 		assert.True(t, strings.HasSuffix(path, string(os.PathSeparator)+name))
+
 	}
 	return proc
 }
@@ -47,6 +50,23 @@ func TestFindProcess(t *testing.T) {
 	testFindProcess(t, "")
 }
 
+func TestFindProcessGo(t *testing.T) {
+	var exe = "go-ps.test"
+	if runtime.GOOS == "windows" {
+		exe += ".exe"
+	}
+	testFindProcess(t, exe)
+}
+
 func TestProcesses(t *testing.T) {
 	testProcesses(t, "")
 }
+
+func TestFindProcessesWithFnError(t *testing.T) {
+	ps, err := findProcessesWithFn(func() ([]Process, error) { return nil, errors.New("TestFindProcessesWithFn Error") }, nil, 0)
+	require.Nil(t, ps)
+	require.NotNil(t, err)
+	ps, err = findProcessesWithFn(func() ([]Process, error) { return nil, nil }, nil, 0)
+	require.Nil(t, ps)
+	require.Nil(t, err)
+}

process_unix_test.go

@@ -2,8 +2,28 @@
 
 package ps
 
-import "testing"
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
 
 func TestUnixProcess(t *testing.T) {
 	var _ Process = new(UnixProcess)
 }
+
+func TestProcessesUnixError(t *testing.T) {
+	proc, err := findProcess(-1)
+	assert.Nil(t, proc)
+	assert.Nil(t, err)
+}
+
+func TestProcessesUnixPPid(t *testing.T) {
+	proc, err := FindProcess(os.Getpid())
+	require.NoError(t, err)
+	require.NotNil(t, proc)
+	assert.Equal(t, os.Getpid(), proc.Pid())
+	assert.Equal(t, os.Getppid(), proc.PPid())
+}