From 43d6eb7173e0402f395df39f0c4de8615c325787 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Sat, 8 Oct 2022 19:28:51 +0200
Subject: [PATCH] pkg/pidfile: remove PIDFile type, rename New() to Write()

This type felt really redundant; `pidfile.New()` takes the path of the file to
create as an argument, so this is already known. The only thing the PIDFile
type provided was a `Remove()` method, which was just calling `os.Remove()` on
the path of the file.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cmd/dockerd/daemon.go       |  5 ++---
 pkg/pidfile/pidfile.go      | 27 +++++++--------------------
 pkg/pidfile/pidfile_test.go | 26 ++++----------------------
 3 files changed, 13 insertions(+), 45 deletions(-)

diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go
index b1d19bc407..3695af0d9e 100644
--- a/cmd/dockerd/daemon.go
+++ b/cmd/dockerd/daemon.go
@@ -139,13 +139,12 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) {
 	potentiallyUnderRuntimeDir := []string{cli.Config.ExecRoot}
 
 	if cli.Pidfile != "" {
-		pf, err := pidfile.New(cli.Pidfile)
-		if err != nil {
+		if err := pidfile.Write(cli.Pidfile); err != nil {
 			return errors.Wrap(err, "failed to start daemon")
 		}
 		potentiallyUnderRuntimeDir = append(potentiallyUnderRuntimeDir, cli.Pidfile)
 		defer func() {
-			if err := pf.Remove(); err != nil {
+			if err := os.Remove(cli.Pidfile); err != nil {
 				logrus.Error(err)
 			}
 		}()
diff --git a/pkg/pidfile/pidfile.go b/pkg/pidfile/pidfile.go
index 04c68e20a0..14773b653f 100644
--- a/pkg/pidfile/pidfile.go
+++ b/pkg/pidfile/pidfile.go
@@ -13,11 +13,6 @@ import (
 	"github.com/docker/docker/pkg/system"
 )
 
-// PIDFile is a file used to store the process ID of a running process.
-type PIDFile struct {
-	path string
-}
-
 func checkPIDFileAlreadyExists(path string) error {
 	pidByte, err := os.ReadFile(path)
 	if err != nil {
@@ -33,23 +28,15 @@ func checkPIDFileAlreadyExists(path string) error {
 	return nil
 }
 
-// New creates a PIDfile using the specified path.
-func New(path string) (*PIDFile, error) {
+// Write writes a "PID file" at the specified path. It returns an error if the
+// file exists and contains a valid PID of a running process, or when failing
+// to write the file.
+func Write(path string) error {
 	if err := checkPIDFileAlreadyExists(path); err != nil {
-		return nil, err
+		return err
 	}
-	// Note MkdirAll returns nil if a directory already exists
 	if err := system.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		return nil, err
+		return err
 	}
-	if err := os.WriteFile(path, []byte(strconv.Itoa(os.Getpid())), 0o644); err != nil {
-		return nil, err
-	}
-
-	return &PIDFile{path: path}, nil
-}
-
-// Remove removes the PIDFile.
-func (file PIDFile) Remove() error {
-	return os.Remove(file.path)
+	return os.WriteFile(path, []byte(strconv.Itoa(os.Getpid())), 0o644)
 }
diff --git a/pkg/pidfile/pidfile_test.go b/pkg/pidfile/pidfile_test.go
index 59860350a7..df3169acdb 100644
--- a/pkg/pidfile/pidfile_test.go
+++ b/pkg/pidfile/pidfile_test.go
@@ -1,37 +1,19 @@
 package pidfile // import "github.com/docker/docker/pkg/pidfile"
 
 import (
-	"os"
 	"path/filepath"
 	"testing"
 )
 
-func TestNewAndRemove(t *testing.T) {
-	dir, err := os.MkdirTemp(os.TempDir(), "test-pidfile")
-	if err != nil {
-		t.Fatal("Could not create test directory")
-	}
-
-	path := filepath.Join(dir, "testfile")
-	file, err := New(path)
+func TestWrite(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "testfile")
+	err := Write(path)
 	if err != nil {
 		t.Fatal("Could not create test file", err)
 	}
 
-	_, err = New(path)
+	err = Write(path)
 	if err == nil {
 		t.Fatal("Test file creation not blocked")
 	}
-
-	if err := file.Remove(); err != nil {
-		t.Fatal("Could not delete created test file")
-	}
-}
-
-func TestRemoveInvalidPath(t *testing.T) {
-	file := PIDFile{path: filepath.Join("foo", "bar")}
-
-	if err := file.Remove(); err == nil {
-		t.Fatal("Non-existing file doesn't give an error on delete")
-	}
 }