cio: openFifos() use named return variables to use in defer()

thaJeztah · thaJeztah · commit baf6c1d5e256 · 2021-02-01T11:17:54.000+01:00
This change is mostly defensive; when checking for the returned
error, it's easy to make a mistake, and check for a "local" error,
not the actual returned error.

This patch changes the function to use a named return variable,
which is checked in the defer.

Signed-off-by: Sebastiaan van Stijn &lt;github@gone.nl&gt;
diff --git a/cio/io_unix.go b/cio/io_unix.go
@@ -103,38 +103,36 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
 	}, nil
 }
 
-func openFifos(ctx context.Context, fifos *FIFOSet) (pipes, error) {
-	var err error
+func openFifos(ctx context.Context, fifos *FIFOSet) (f pipes, retErr error) {
 	defer func() {
-		if err != nil {
+		if retErr != nil {
 			fifos.Close()
 		}
 	}()
 
-	var f pipes
 	if fifos.Stdin != "" {
-		if f.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil {
-			return f, errors.Wrapf(err, "failed to open stdin fifo")
+		if f.Stdin, retErr = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
+			return f, errors.Wrapf(retErr, "failed to open stdin fifo")
 		}
 		defer func() {
-			if err != nil && f.Stdin != nil {
+			if retErr != nil && f.Stdin != nil {
 				f.Stdin.Close()
 			}
 		}()
 	}
 	if fifos.Stdout != "" {
-		if f.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil {
-			return f, errors.Wrapf(err, "failed to open stdout fifo")
+		if f.Stdout, retErr = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
+			return f, errors.Wrapf(retErr, "failed to open stdout fifo")
 		}
 		defer func() {
-			if err != nil && f.Stdout != nil {
+			if retErr != nil && f.Stdout != nil {
 				f.Stdout.Close()
 			}
 		}()
 	}
 	if !fifos.Terminal && fifos.Stderr != "" {
-		if f.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil {
-			return f, errors.Wrapf(err, "failed to open stderr fifo")
+		if f.Stderr, retErr = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
+			return f, errors.Wrapf(retErr, "failed to open stderr fifo")
 		}
 	}
 	return f, nil

Original file line number	Diff line number	Diff line change
`@@ -103,38 +103,36 @@ func copyIO(fifos FIFOSet, ioset Streams) (*cio, error) {`
`103`	`103`	`}, nil`
`104`	`104`	`}`
`105`	`105`
`106`		`-func openFifos(ctx context.Context, fifos *FIFOSet) (pipes, error) {`
`107`		`- var err error`
	`106`	`+func openFifos(ctx context.Context, fifos *FIFOSet) (f pipes, retErr error) {`
`108`	`107`	`defer func() {`
`109`		`- if err != nil {`
	`108`	`+ if retErr != nil {`
`110`	`109`	`fifos.Close()`
`111`	`110`	`}`
`112`	`111`	`}()`
`113`	`112`
`114`		`- var f pipes`
`115`	`113`	`if fifos.Stdin != "" {`
`116`		`- if f.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY\|syscall.O_CREAT\|syscall.O_NONBLOCK, 0700); err != nil {`
`117`		`- return f, errors.Wrapf(err, "failed to open stdin fifo")`
	`114`	`+ if f.Stdin, retErr = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY\|syscall.O_CREAT\|syscall.O_NONBLOCK, 0700); retErr != nil {`
	`115`	`+ return f, errors.Wrapf(retErr, "failed to open stdin fifo")`
`118`	`116`	`}`
`119`	`117`	`defer func() {`
`120`		`- if err != nil && f.Stdin != nil {`
	`118`	`+ if retErr != nil && f.Stdin != nil {`
`121`	`119`	`f.Stdin.Close()`
`122`	`120`	`}`
`123`	`121`	`}()`
`124`	`122`	`}`
`125`	`123`	`if fifos.Stdout != "" {`
`126`		`- if f.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY\|syscall.O_CREAT\|syscall.O_NONBLOCK, 0700); err != nil {`
`127`		`- return f, errors.Wrapf(err, "failed to open stdout fifo")`
	`124`	`+ if f.Stdout, retErr = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY\|syscall.O_CREAT\|syscall.O_NONBLOCK, 0700); retErr != nil {`
	`125`	`+ return f, errors.Wrapf(retErr, "failed to open stdout fifo")`
`128`	`126`	`}`
`129`	`127`	`defer func() {`
`130`		`- if err != nil && f.Stdout != nil {`
	`128`	`+ if retErr != nil && f.Stdout != nil {`
`131`	`129`	`f.Stdout.Close()`
`132`	`130`	`}`
`133`	`131`	`}()`
`134`	`132`	`}`
`135`	`133`	`if !fifos.Terminal && fifos.Stderr != "" {`
`136`		`- if f.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY\|syscall.O_CREAT\|syscall.O_NONBLOCK, 0700); err != nil {`
`137`		`- return f, errors.Wrapf(err, "failed to open stderr fifo")`
	`134`	`+ if f.Stderr, retErr = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY\|syscall.O_CREAT\|syscall.O_NONBLOCK, 0700); retErr != nil {`
	`135`	`+ return f, errors.Wrapf(retErr, "failed to open stderr fifo")`
`138`	`136`	`}`
`139`	`137`	`}`
`140`	`138`	`return f, nil`