Remove all instances of named return "err" from Libpod

This was inspired by https://github.com/cri-o/cri-o/pull/3934 and
much of the logic for it is contained there. However, in brief,
a named return called "err" can cause lots of code confusion and
encourages using the wrong err variable in defer statements,
which can make them work incorrectly. Using a separate name which
is not used elsewhere makes it very clear what the defer should
be doing.

As part of this, remove a large number of named returns that were
not used anywhere. Most of them were once needed, but are no
longer necessary after previous refactors (but were accidentally
retained).

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

1 files changed, 22 insertions, 22 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 12132cf41..e98e20b9b 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -210,7 +210,7 @@ func (c *Container) handleExitFile(exitFile string, fi os.FileInfo) error {
 // Handle container restart policy.
 // This is called when a container has exited, and was not explicitly stopped by
 // an API call to stop the container or pod it is in.
-func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, err error) {
+func (c *Container) handleRestartPolicy(ctx context.Context) (_ bool, retErr error) {
 	// If we did not get a restart policy match, exit immediately.
 	// Do the same if we're not a policy that restarts.
 	if !c.state.RestartPolicyMatch ||
@@ -241,7 +241,7 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, er
 	logrus.Debugf("Restarting container %s due to restart policy %s", c.ID(), c.config.RestartPolicy)
 
 	// Need to check if dependencies are alive.
-	if err = c.checkDependenciesAndHandleError(); err != nil {
+	if err := c.checkDependenciesAndHandleError(); err != nil {
 		return false, err
 	}
 
@@ -263,9 +263,9 @@ func (c *Container) handleRestartPolicy(ctx context.Context) (restarted bool, er
 	}
 
 	defer func() {
-		if err != nil {
-			if err2 := c.cleanup(ctx); err2 != nil {
-				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
+		if retErr != nil {
+			if err := c.cleanup(ctx); err != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err)
 			}
 		}
 	}()
@@ -768,7 +768,7 @@ func (c *Container) save() error {
 // Checks the container is in the right state, then initializes the container in preparation to start the container.
 // If recursive is true, each of the containers dependencies will be started.
 // Otherwise, this function will return with error if there are dependencies of this container that aren't running.
-func (c *Container) prepareToStart(ctx context.Context, recursive bool) (err error) {
+func (c *Container) prepareToStart(ctx context.Context, recursive bool) (retErr error) {
 	// Container must be created or stopped to be started
 	if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateStopped, define.ContainerStateExited) {
 		return errors.Wrapf(define.ErrCtrStateInvalid, "container %s must be in Created or Stopped state to be started", c.ID())
@@ -785,9 +785,9 @@ func (c *Container) prepareToStart(ctx context.Context, recursive bool) (err err
 	}
 
 	defer func() {
-		if err != nil {
-			if err2 := c.cleanup(ctx); err2 != nil {
-				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
+		if retErr != nil {
+			if err := c.cleanup(ctx); err != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err)
 			}
 		}
 	}()
@@ -1133,7 +1133,7 @@ func (c *Container) reinit(ctx context.Context, retainRetries bool) error {
 // Initialize (if necessary) and start a container
 // Performs all necessary steps to start a container that is not running
 // Does not lock or check validity
-func (c *Container) initAndStart(ctx context.Context) (err error) {
+func (c *Container) initAndStart(ctx context.Context) (retErr error) {
 	// If we are ContainerStateUnknown, throw an error
 	if c.state.State == define.ContainerStateUnknown {
 		return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is in an unknown state", c.ID())
@@ -1151,9 +1151,9 @@ func (c *Container) initAndStart(ctx context.Context) (err error) {
 	}
 
 	defer func() {
-		if err != nil {
-			if err2 := c.cleanup(ctx); err2 != nil {
-				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
+		if retErr != nil {
+			if err := c.cleanup(ctx); err != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err)
 			}
 		}
 	}()
@@ -1335,7 +1335,7 @@ func (c *Container) unpause() error {
 }
 
 // Internal, non-locking function to restart a container
-func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err error) {
+func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (retErr error) {
 	if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateCreated, define.ContainerStateRunning, define.ContainerStateStopped, define.ContainerStateExited) {
 		return errors.Wrapf(define.ErrCtrStateInvalid, "unable to restart a container in a paused or unknown state")
 	}
@@ -1372,9 +1372,9 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e
 		}
 	}
 	defer func() {
-		if err != nil {
-			if err2 := c.cleanup(ctx); err2 != nil {
-				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err2)
+		if retErr != nil {
+			if err := c.cleanup(ctx); err != nil {
+				logrus.Errorf("error cleaning up container %s: %v", c.ID(), err)
 			}
 		}
 	}()
@@ -1696,7 +1696,7 @@ func (c *Container) cleanup(ctx context.Context) error {
 
 // delete deletes the container and runs any configured poststop
 // hooks.
-func (c *Container) delete(ctx context.Context) (err error) {
+func (c *Container) delete(ctx context.Context) error {
 	span, _ := opentracing.StartSpanFromContext(ctx, "delete")
 	span.SetTag("struct", "container")
 	defer span.Finish()
@@ -1715,7 +1715,7 @@ func (c *Container) delete(ctx context.Context) (err error) {
 // postDeleteHooks runs the poststop hooks (if any) as specified by
 // the OCI Runtime Specification (which requires them to run
 // post-delete, despite the stage name).
-func (c *Container) postDeleteHooks(ctx context.Context) (err error) {
+func (c *Container) postDeleteHooks(ctx context.Context) error {
 	span, _ := opentracing.StartSpanFromContext(ctx, "postDeleteHooks")
 	span.SetTag("struct", "container")
 	defer span.Finish()
@@ -1838,7 +1838,7 @@ func (c *Container) saveSpec(spec *spec.Spec) error {
 }
 
 // Warning: precreate hooks may alter 'config' in place.
-func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (extensionStageHooks map[string][]spec.Hook, err error) {
+func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[string][]spec.Hook, error) {
 	allHooks := make(map[string][]spec.Hook)
 	if c.runtime.config.Engine.HooksDir == nil {
 		if rootless.IsRootless() {
@@ -1952,7 +1952,7 @@ func (c *Container) checkReadyForRemoval() error {
 
 // writeJSONFile marshalls and writes the given data to a JSON file
 // in the bundle path
-func (c *Container) writeJSONFile(v interface{}, file string) (err error) {
+func (c *Container) writeJSONFile(v interface{}, file string) error {
 	fileJSON, err := json.MarshalIndent(v, "", "  ")
 	if err != nil {
 		return errors.Wrapf(err, "error writing JSON to %s for container %s", file, c.ID())
@@ -1967,7 +1967,7 @@ func (c *Container) writeJSONFile(v interface{}, file string) (err error) {
 
 // prepareCheckpointExport writes the config and spec to
 // JSON files for later export
-func (c *Container) prepareCheckpointExport() (err error) {
+func (c *Container) prepareCheckpointExport() error {
 	// save live config
 	if err := c.writeJSONFile(c.Config(), "config.dump"); err != nil {
 		return err