From 4a3785dddeb72ec369950ee0dc163d8f58f934bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Thu, 18 Apr 2024 00:47:09 +0200 Subject: [PATCH] Refactor the error handling further MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use defer, a nested function, and early returns. Besides being a bit more directly related to what we want to achieve, this now does not call decompressed.Close() on a nil value if DecompressStream fails. Signed-off-by: Miloslav Trmač --- pkg/blobcache/dest.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/pkg/blobcache/dest.go b/pkg/blobcache/dest.go index 2ac618312..fb2efe4c4 100644 --- a/pkg/blobcache/dest.go +++ b/pkg/blobcache/dest.go @@ -79,6 +79,7 @@ func (d *blobCacheDestination) IgnoresEmbeddedDockerReference() bool { // and this new file. func (d *blobCacheDestination) saveStream(wg *sync.WaitGroup, decompressReader io.ReadCloser, tempFile *os.File, compressedFilename string, compressedDigest digest.Digest, isConfig bool, alternateDigest *digest.Digest) { defer wg.Done() + defer decompressReader.Close() succeeded := false defer func() { @@ -90,28 +91,30 @@ func (d *blobCacheDestination) saveStream(wg *sync.WaitGroup, decompressReader i } }() - // Decompress from and digest the reading end of that pipe. - decompressed, err3 := archive.DecompressStream(decompressReader) digester := digest.Canonical.Digester() - if err3 == nil { + if err := func() error { // A scope for defer + defer tempFile.Close() + + // Decompress from and digest the reading end of that pipe. + decompressed, err := archive.DecompressStream(decompressReader) + if err != nil { + // Drain the pipe to keep from stalling the PutBlob() thread. + if _, err2 := io.Copy(io.Discard, decompressReader); err2 != nil { + logrus.Debugf("error draining the pipe: %v", err2) + } + return err + } + defer decompressed.Close() // Read the decompressed data through the filter over the pipe, blocking until the // writing end is closed. - _, err3 = io.Copy(io.MultiWriter(tempFile, digester.Hash()), decompressed) - } else { - // Drain the pipe to keep from stalling the PutBlob() thread. - if _, err := io.Copy(io.Discard, decompressReader); err != nil { - logrus.Debugf("error draining the pipe: %v", err) - } + _, err = io.Copy(io.MultiWriter(tempFile, digester.Hash()), decompressed) + return err + }(); err != nil { + return } - decompressReader.Close() - decompressed.Close() - tempFile.Close() // Determine the name that we should give to the uncompressed copy of the blob. decompressedFilename := d.reference.blobPath(digester.Digest(), isConfig) - if err3 != nil { - return - } // Rename the temporary file. if err := os.Rename(tempFile.Name(), decompressedFilename); err != nil { logrus.Debugf("error renaming new decompressed copy of blob %q into place at %q: %v", digester.Digest().String(), decompressedFilename, err)