v1: reduce duplicated code

Code for handling Tasks and Processes, except for "cgroup.procs" vs "tasks",
and Tasks being a different type.

This patch:

- Makes the Task type an alias for Process (as they're identical otherwise)
- Adds a `processType` type for the `cgroupProcs` and `cgroupTasks` consts. This type
  is an alias for "string", and mostly for clarity (indicating it's an 'enum').
- Merges the `cgroup.add()` and `cgroup.addTask()` functions, adding a `pType` argument.
- Merges the `cgroup.processes()` and `cgroup.tasks()` functions, adding a `pType` argument.
- Merges the `readPids()` and `readTasksPids()` utilities, adding a `pType` argument.
- Move locking and validation into `cgroup.add()`. All places using `cgroup.add()`
  were taking a lock and doing this validation, so looks we can move this code into
  `cgroup.add()` itself.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cgroup.go

@@ -151,46 +151,20 @@ func (c *cgroup) Subsystems() []Subsystem {
 
 // Add moves the provided process into the new cgroup
 func (c *cgroup) Add(process Process) error {
-	if process.Pid <= 0 {
-		return ErrInvalidPid
-	}
-	c.mu.Lock()
-	defer c.mu.Unlock()
-	if c.err != nil {
-		return c.err
-	}
-	return c.add(process)
+	return c.add(process, cgroupProcs)
 }
 
 // AddProc moves the provided process id into the new cgroup
 func (c *cgroup) AddProc(pid uint64) error {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-	if c.err != nil {
-		return c.err
-	}
-	return c.Add(Process{Pid: int(pid)})
-}
-
-func (c *cgroup) add(process Process) error {
-	for _, s := range pathers(c.subsystems) {
-		p, err := c.path(s.Name())
-		if err != nil {
-			return err
-		}
-		if err := retryingWriteFile(
-			filepath.Join(s.Path(p), cgroupProcs),
-			[]byte(strconv.Itoa(process.Pid)),
-			defaultFilePerm,
-		); err != nil {
-			return err
-		}
-	}
-	return nil
+	return c.add(Process{Pid: int(pid)}, cgroupProcs)
 }
 
 // AddTask moves the provided tasks (threads) into the new cgroup
 func (c *cgroup) AddTask(process Process) error {
+	return c.add(process, cgroupTasks)
+}
+
+func (c *cgroup) add(process Process, pType procType) error {
 	if process.Pid <= 0 {
 		return ErrInvalidPid
 	}
@@ -199,20 +173,17 @@ func (c *cgroup) AddTask(process Process) error {
 	if c.err != nil {
 		return c.err
 	}
-	return c.addTask(process)
-}
-
-func (c *cgroup) addTask(process Process) error {
 	for _, s := range pathers(c.subsystems) {
 		p, err := c.path(s.Name())
 		if err != nil {
 			return err
 		}
-		if err := retryingWriteFile(
-			filepath.Join(s.Path(p), cgroupTasks),
+		err = retryingWriteFile(
+			filepath.Join(s.Path(p), pType),
 			[]byte(strconv.Itoa(process.Pid)),
 			defaultFilePerm,
-		); err != nil {
+		)
+		if err != nil {
 			return err
 		}
 	}
@@ -336,39 +307,7 @@ func (c *cgroup) Processes(subsystem Name, recursive bool) ([]Process, error) {
 	if c.err != nil {
 		return nil, c.err
 	}
-	return c.processes(subsystem, recursive)
-}
-
-func (c *cgroup) processes(subsystem Name, recursive bool) ([]Process, error) {
-	s := c.getSubsystem(subsystem)
-	sp, err := c.path(subsystem)
-	if err != nil {
-		return nil, err
-	}
-	path := s.(pather).Path(sp)
-	var processes []Process
-	err = filepath.Walk(path, func(p string, info os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if !recursive && info.IsDir() {
-			if p == path {
-				return nil
-			}
-			return filepath.SkipDir
-		}
-		dir, name := filepath.Split(p)
-		if name != cgroupProcs {
-			return nil
-		}
-		procs, err := readPids(dir, subsystem)
-		if err != nil {
-			return err
-		}
-		processes = append(processes, procs...)
-		return nil
-	})
-	return processes, err
+	return c.processes(subsystem, recursive, cgroupProcs)
 }
 
 // Tasks returns the tasks running inside the cgroup along
@@ -379,17 +318,17 @@ func (c *cgroup) Tasks(subsystem Name, recursive bool) ([]Task, error) {
 	if c.err != nil {
 		return nil, c.err
 	}
-	return c.tasks(subsystem, recursive)
+	return c.processes(subsystem, recursive, cgroupTasks)
 }
 
-func (c *cgroup) tasks(subsystem Name, recursive bool) ([]Task, error) {
+func (c *cgroup) processes(subsystem Name, recursive bool, pType procType) ([]Process, error) {
 	s := c.getSubsystem(subsystem)
 	sp, err := c.path(subsystem)
 	if err != nil {
 		return nil, err
 	}
 	path := s.(pather).Path(sp)
-	var tasks []Task
+	var processes []Process
 	err = filepath.Walk(path, func(p string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -401,17 +340,17 @@ func (c *cgroup) tasks(subsystem Name, recursive bool) ([]Task, error) {
 			return filepath.SkipDir
 		}
 		dir, name := filepath.Split(p)
-		if name != cgroupTasks {
+		if name != pType {
 			return nil
 		}
-		procs, err := readTasksPids(dir, subsystem)
+		procs, err := readPids(dir, subsystem, pType)
 		if err != nil {
 			return err
 		}
-		tasks = append(tasks, procs...)
+		processes = append(processes, procs...)
 		return nil
 	})
-	return tasks, err
+	return processes, err
 }
 
 // Freeze freezes the entire cgroup and all the processes inside it
@@ -521,7 +460,7 @@ func (c *cgroup) MoveTo(destination Cgroup) error {
 		return c.err
 	}
 	for _, s := range c.subsystems {
-		processes, err := c.processes(s.Name(), true)
+		processes, err := c.processes(s.Name(), true, cgroupProcs)
 		if err != nil {
 			return err
 		}

control.go

@@ -23,10 +23,12 @@ import (
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 )
 
+type procType = string
+
 const (
-	cgroupProcs    = "cgroup.procs"
-	cgroupTasks    = "tasks"
-	defaultDirPerm = 0755
+	cgroupProcs    procType = "cgroup.procs"
+	cgroupTasks    procType = "tasks"
+	defaultDirPerm          = 0755
 )
 
 // defaultFilePerm is a var so that the test framework can change the filemode
@@ -37,22 +39,15 @@ const (
 var defaultFilePerm = os.FileMode(0)
 
 type Process struct {
-	// Subsystem is the name of the subsystem that the process is in
+	// Subsystem is the name of the subsystem that the process / task is in.
 	Subsystem Name
-	// Pid is the process id of the process
+	// Pid is the process id of the process / task.
 	Pid int
-	// Path is the full path of the subsystem and location that the process is in
+	// Path is the full path of the subsystem and location that the process / task is in.
 	Path string
 }
 
-type Task struct {
-	// Subsystem is the name of the subsystem that the task is in
-	Subsystem Name
-	// Pid is the process id of the task
-	Pid int
-	// Path is the full path of the subsystem and location that the task is in
-	Path string
-}
+type Task = Process
 
 // Cgroup handles interactions with the individual groups to perform
 // actions on them as them main interface to this cgroup package

utils.go

@@ -164,9 +164,9 @@ func remove(path string) error {
 	return fmt.Errorf("cgroups: unable to remove path %q", path)
 }
 
-// readPids will read all the pids of processes in a cgroup by the provided path
-func readPids(path string, subsystem Name) ([]Process, error) {
-	f, err := os.Open(filepath.Join(path, cgroupProcs))
+// readPids will read all the pids of processes or tasks in a cgroup by the provided path
+func readPids(path string, subsystem Name, pType procType) ([]Process, error) {
+	f, err := os.Open(filepath.Join(path, pType))
 	if err != nil {
 		return nil, err
 	}
@@ -195,36 +195,6 @@ func readPids(path string, subsystem Name) ([]Process, error) {
 	return out, nil
 }
 
-// readTasksPids will read all the pids of tasks in a cgroup by the provided path
-func readTasksPids(path string, subsystem Name) ([]Task, error) {
-	f, err := os.Open(filepath.Join(path, cgroupTasks))
-	if err != nil {
-		return nil, err
-	}
-	defer f.Close()
-	var (
-		out []Task
-		s   = bufio.NewScanner(f)
-	)
-	for s.Scan() {
-		if t := s.Text(); t != "" {
-			pid, err := strconv.Atoi(t)
-			if err != nil {
-				return nil, err
-			}
-			out = append(out, Task{
-				Pid:       pid,
-				Subsystem: subsystem,
-				Path:      path,
-			})
-		}
-	}
-	if err := s.Err(); err != nil {
-		return nil, err
-	}
-	return out, nil
-}
-
 func hugePageSizes() ([]string, error) {
 	var (
 		pageSizes []string