From b98c288d068c1278431a41883222a04420c1d54f Mon Sep 17 00:00:00 2001 From: Yaz Saito Date: Wed, 12 Feb 2020 23:57:54 -0800 Subject: [PATCH 1/2] Report potential errors during Close and Chmod. In practice it should be safe to ignore errors from Close() on most Unix-like platforms. But its better to be paranoid. --- copy.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/copy.go b/copy.go index 52afac4..59688b3 100644 --- a/copy.go +++ b/copy.go @@ -39,9 +39,13 @@ func copy(src, dest string, info os.FileInfo) error { // fcopy is for just a file, // with considering existence of parent directory // and file permission. -func fcopy(src, dest string, info os.FileInfo) error { - - if err := os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil { +func fcopy(src, dest string, info os.FileInfo) (err error) { + close := func(f *os.File) { + if e := f.Close(); e != nil && err == nil { + err = e + } + } + if err = os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil { return err } @@ -49,7 +53,7 @@ func fcopy(src, dest string, info os.FileInfo) error { if err != nil { return err } - defer f.Close() + defer close(f) if err = os.Chmod(f.Name(), info.Mode()); err != nil { return err @@ -59,7 +63,7 @@ func fcopy(src, dest string, info os.FileInfo) error { if err != nil { return err } - defer s.Close() + defer close(s) _, err = io.Copy(f, s) return err @@ -68,16 +72,20 @@ func fcopy(src, dest string, info os.FileInfo) error { // dcopy is for a directory, // with scanning contents inside the directory // and pass everything to "copy" recursively. -func dcopy(srcdir, destdir string, info os.FileInfo) error { +func dcopy(srcdir, destdir string, info os.FileInfo) (err error) { originalMode := info.Mode() // Make dest dir with 0755 so that everything writable. - if err := os.MkdirAll(destdir, tmpPermissionForDirectory); err != nil { + if err = os.MkdirAll(destdir, tmpPermissionForDirectory); err != nil { return err } // Recover dir mode with original one. - defer os.Chmod(destdir, originalMode) + defer func() { + if e := os.Chmod(destdir, originalMode); e != nil && err == nil { + err = e + } + }() contents, err := ioutil.ReadDir(srcdir) if err != nil { @@ -86,7 +94,7 @@ func dcopy(srcdir, destdir string, info os.FileInfo) error { for _, content := range contents { cs, cd := filepath.Join(srcdir, content.Name()), filepath.Join(destdir, content.Name()) - if err := copy(cs, cd, content); err != nil { + if err = copy(cs, cd, content); err != nil { // If any error, exit immediately return err } From 9b34c523de317b394b8cace1fc3986d59bf340ed Mon Sep 17 00:00:00 2001 From: Hiromu Ochiai Date: Fri, 14 Feb 2020 11:52:34 +0900 Subject: [PATCH 2/2] Separate util functions --- copy.go | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/copy.go b/copy.go index 59688b3..c9f14c5 100644 --- a/copy.go +++ b/copy.go @@ -40,12 +40,8 @@ func copy(src, dest string, info os.FileInfo) error { // with considering existence of parent directory // and file permission. func fcopy(src, dest string, info os.FileInfo) (err error) { - close := func(f *os.File) { - if e := f.Close(); e != nil && err == nil { - err = e - } - } - if err = os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil { + + if err := os.MkdirAll(filepath.Dir(dest), os.ModePerm); err != nil { return err } @@ -53,7 +49,7 @@ func fcopy(src, dest string, info os.FileInfo) (err error) { if err != nil { return err } - defer close(f) + defer fclose(f, &err) if err = os.Chmod(f.Name(), info.Mode()); err != nil { return err @@ -63,7 +59,7 @@ func fcopy(src, dest string, info os.FileInfo) (err error) { if err != nil { return err } - defer close(s) + defer fclose(s, &err) _, err = io.Copy(f, s) return err @@ -77,15 +73,11 @@ func dcopy(srcdir, destdir string, info os.FileInfo) (err error) { originalMode := info.Mode() // Make dest dir with 0755 so that everything writable. - if err = os.MkdirAll(destdir, tmpPermissionForDirectory); err != nil { + if err := os.MkdirAll(destdir, tmpPermissionForDirectory); err != nil { return err } // Recover dir mode with original one. - defer func() { - if e := os.Chmod(destdir, originalMode); e != nil && err == nil { - err = e - } - }() + defer chmod(destdir, originalMode, &err) contents, err := ioutil.ReadDir(srcdir) if err != nil { @@ -94,7 +86,7 @@ func dcopy(srcdir, destdir string, info os.FileInfo) (err error) { for _, content := range contents { cs, cd := filepath.Join(srcdir, content.Name()), filepath.Join(destdir, content.Name()) - if err = copy(cs, cd, content); err != nil { + if err := copy(cs, cd, content); err != nil { // If any error, exit immediately return err } @@ -112,3 +104,19 @@ func lcopy(src, dest string, info os.FileInfo) error { } return os.Symlink(src, dest) } + +// fclose ANYHOW closes file, +// with asiging error occured BUT respecting the error already reported. +func fclose(f *os.File, reported *error) { + if err := f.Close(); *reported == nil { + *reported = err + } +} + +// chmod ANYHOW changes file mode, +// with asiging error occured BUT respecting the error already reported. +func chmod(dir string, mode os.FileMode, reported *error) { + if err := os.Chmod(dir, mode); *reported == nil { + *reported = err + } +}