diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 537e1c5c5..85b8f202b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -241,7 +241,7 @@ jobs: uses: golangci/golangci-lint-action@v2 with: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.29 + version: v1.36 # Optional: show only new issues if it's a pull request. The default value is `false`. only-new-issues: true args: --verbose --timeout 5m diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..55ad1a266 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,57 @@ +# This is the configuration for golangci-lint for the restic project. +# +# A sample config with all settings is here: +# https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml + +linters: + # only enable the linters listed below + disable-all: true + enable: + # make sure all errors returned by functions are handled + - errcheck + + # find unused code + - deadcode + + # show how code can be simplified + - gosimple + + # # make sure code is formatted + - gofmt + + # examine code and report suspicious constructs, such as Printf calls whose + # arguments do not align with the format string + - govet + + # make sure names and comments are used according to the conventions + - golint + + # detect when assignments to existing variables are not used + - ineffassign + + # run static analysis and find errors + - staticcheck + + # find unused variables, functions, structs, types, etc. + - unused + + # find unused struct fields + - structcheck + + # find unused global variables + - varcheck + + # parse and typecheck code + - typecheck + +issues: + # don't use the default exclude rules, this hides (among others) ignored + # errors from Close() calls + exclude-use-default: false + + # list of things to not warn about + exclude: + # golint: do not warn about missing comments for exported stuff + - exported (function|method|var|type|const) `.*` should have comment or be unexported + # golint: ignore constants in all caps + - don't use ALL_CAPS in Go names; use CamelCase diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c63dc0d72..903e4459f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -141,6 +141,14 @@ Installing the script `fmt-check` from https://github.com/edsrzf/gofmt-git-hook locally as a pre-commit hook checks formatting before committing automatically, just copy this script to `.git/hooks/pre-commit`. +The project is using the program +[`golangci-lint`](https://github.com/golangci/golangci-lint) to run a list of +linters and checkers. It will be run on the code when you submit a PR. In order +to check your code beforehand, you can run `golangci-lint run` manually. +Eventually, we will enable `golangci-lint` for the whole code base. For now, +you can ignore warnings printed for lines you did not modify, those will be +ignored by the CI. + For each pull request, several different systems run the integration tests on Linux, macOS and Windows. We won't merge any code that does not pass all tests for all systems, so when a tests fails, try to find out what's wrong and fix diff --git a/changelog/unreleased/pr-3250 b/changelog/unreleased/pr-3250 new file mode 100644 index 000000000..f4aa555e5 --- /dev/null +++ b/changelog/unreleased/pr-3250 @@ -0,0 +1,6 @@ +Enhancement: Add more error checks + +We've added a lot more error checks in places where errors were ignored before +(as hinted by the static analysis programm `errcheck` via `golangci-lint`). + +https://github.com/restic/restic/pull/3250 diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index d8500b263..bd8b4da8c 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -119,7 +119,11 @@ func init() { f.StringVarP(&backupOptions.Host, "host", "H", "", "set the `hostname` for the snapshot manually. To prevent an expensive rescan use the \"parent\" flag") f.StringVar(&backupOptions.Host, "hostname", "", "set the `hostname` for the snapshot manually") - f.MarkDeprecated("hostname", "use --host") + err := f.MarkDeprecated("hostname", "use --host") + if err != nil { + // MarkDeprecated only returns an error when the flag could not be found + panic(err) + } f.StringArrayVar(&backupOptions.FilesFrom, "files-from", nil, "read the files to backup from `file` (can be combined with file args; can be specified multiple times)") f.StringArrayVar(&backupOptions.FilesFromVerbatim, "files-from-verbatim", nil, "read the files to backup from `file` (can be combined with file args; can be specified multiple times)") @@ -201,10 +205,21 @@ func readFilenamesFromFileRaw(filename string) (names []string, err error) { if f, err = os.Open(filename); err != nil { return nil, err } - defer f.Close() } - return readFilenamesRaw(f) + names, err = readFilenamesRaw(f) + if err != nil { + // ignore subsequent errors + _ = f.Close() + return nil, err + } + + err = f.Close() + if err != nil { + return nil, err + } + + return names, nil } func readFilenamesRaw(r io.Reader) (names []string, err error) { diff --git a/cmd/restic/cmd_backup_test.go b/cmd/restic/cmd_backup_test.go index 90e9a5a4b..49642b872 100644 --- a/cmd/restic/cmd_backup_test.go +++ b/cmd/restic/cmd_backup_test.go @@ -32,7 +32,7 @@ func TestCollectTargets(t *testing.T) { // All mentioned files must exist for collectTargets. f, err := os.Create(filepath.Join(dir, filename)) rtest.OK(t, err) - f.Close() + rtest.OK(t, f.Close()) expect = append(expect, f.Name()) } @@ -41,7 +41,7 @@ func TestCollectTargets(t *testing.T) { rtest.OK(t, err) // Empty lines should be ignored. A line starting with '#' is a comment. fmt.Fprintf(f1, "\n%s*\n # here's a comment\n", f1.Name()) - f1.Close() + rtest.OK(t, f1.Close()) f2, err := os.Create(filepath.Join(dir, "fromfile-verbatim")) rtest.OK(t, err) @@ -49,7 +49,7 @@ func TestCollectTargets(t *testing.T) { // Empty lines should be ignored. CR+LF is allowed. fmt.Fprintf(f2, "%s\r\n\n", filepath.Join(dir, filename)) } - f2.Close() + rtest.OK(t, f2.Close()) f3, err := os.Create(filepath.Join(dir, "fromfile-raw")) rtest.OK(t, err) @@ -57,7 +57,7 @@ func TestCollectTargets(t *testing.T) { fmt.Fprintf(f3, "%s\x00", filepath.Join(dir, filename)) } rtest.OK(t, err) - f3.Close() + rtest.OK(t, f3.Close()) opts := BackupOptions{ FilesFrom: []string{f1.Name()}, diff --git a/cmd/restic/cmd_cache.go b/cmd/restic/cmd_cache.go index 89fb2c0df..1b36219eb 100644 --- a/cmd/restic/cmd_cache.go +++ b/cmd/restic/cmd_cache.go @@ -148,7 +148,7 @@ func runCache(opts CacheOptions, gopts GlobalOptions, args []string) error { }) } - tab.Write(gopts.stdout) + _ = tab.Write(gopts.stdout) Printf("%d cache dirs in %s\n", len(dirs), cachedir) return nil diff --git a/cmd/restic/cmd_find.go b/cmd/restic/cmd_find.go index 0986b5a13..378d6cbdc 100644 --- a/cmd/restic/cmd_find.go +++ b/cmd/restic/cmd_find.go @@ -563,7 +563,10 @@ func runFind(opts FindOptions, gopts GlobalOptions, args []string) error { } if opts.PackID { - f.packsToBlobs(ctx, []string{f.pat.pattern[0]}) // TODO: support multiple packs + err := f.packsToBlobs(ctx, []string{f.pat.pattern[0]}) // TODO: support multiple packs + if err != nil { + return err + } } for sn := range FindFilteredSnapshots(ctx, repo, opts.Hosts, opts.Tags, opts.Paths, opts.Snapshots) { diff --git a/cmd/restic/cmd_forget.go b/cmd/restic/cmd_forget.go index ec90879ec..83b0ac91d 100644 --- a/cmd/restic/cmd_forget.go +++ b/cmd/restic/cmd_forget.go @@ -68,7 +68,11 @@ func init() { f.Var(&forgetOptions.KeepTags, "keep-tag", "keep snapshots with this `taglist` (can be specified multiple times)") f.StringArrayVar(&forgetOptions.Hosts, "host", nil, "only consider snapshots with the given `host` (can be specified multiple times)") f.StringArrayVar(&forgetOptions.Hosts, "hostname", nil, "only consider snapshots with the given `hostname` (can be specified multiple times)") - f.MarkDeprecated("hostname", "use --host") + err := f.MarkDeprecated("hostname", "use --host") + if err != nil { + // MarkDeprecated only returns an error when the flag is not found + panic(err) + } f.Var(&forgetOptions.Tags, "tag", "only consider snapshots which include this `taglist` in the format `tag[,tag,...]` (can be specified multiple times)") diff --git a/cmd/restic/cmd_ls.go b/cmd/restic/cmd_ls.go index db908a72c..90106e8a5 100644 --- a/cmd/restic/cmd_ls.go +++ b/cmd/restic/cmd_ls.go @@ -159,16 +159,17 @@ func runLs(opts LsOptions, gopts GlobalOptions, args []string) error { enc := json.NewEncoder(gopts.stdout) printSnapshot = func(sn *restic.Snapshot) { - enc.Encode(lsSnapshot{ + err = enc.Encode(lsSnapshot{ Snapshot: sn, ID: sn.ID(), ShortID: sn.ID().Str(), StructType: "snapshot", }) + panic(err) } printNode = func(path string, node *restic.Node) { - enc.Encode(lsNode{ + err = enc.Encode(lsNode{ Name: node.Name, Type: node.Type, Path: path, @@ -181,6 +182,7 @@ func runLs(opts LsOptions, gopts GlobalOptions, args []string) error { ChangeTime: node.ChangeTime, StructType: "node", }) + panic(err) } } else { printSnapshot = func(sn *restic.Snapshot) { diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index 8f11b6860..c6b111d22 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -117,7 +117,10 @@ func runRecover(gopts GlobalOptions) error { ModTime: time.Now(), ChangeTime: time.Now(), } - tree.Insert(&node) + err = tree.Insert(&node) + if err != nil { + return err + } } treeID, err := repo.SaveTree(gopts.ctx, tree) diff --git a/cmd/restic/cmd_snapshots.go b/cmd/restic/cmd_snapshots.go index 300ba99ee..091861bfa 100644 --- a/cmd/restic/cmd_snapshots.go +++ b/cmd/restic/cmd_snapshots.go @@ -243,7 +243,10 @@ func PrintSnapshots(stdout io.Writer, list restic.Snapshots, reasons []restic.Ke } } - tab.Write(stdout) + err := tab.Write(stdout) + if err != nil { + Warnf("error printing: %v\n", err) + } } // PrintSnapshotGroupHeader prints which group of the group-by option the diff --git a/cmd/restic/cmd_snapshots_test.go b/cmd/restic/cmd_snapshots_test.go index 04af1a3c1..777c4272f 100644 --- a/cmd/restic/cmd_snapshots_test.go +++ b/cmd/restic/cmd_snapshots_test.go @@ -11,7 +11,8 @@ import ( func TestEmptySnapshotGroupJSON(t *testing.T) { for _, grouped := range []bool{false, true} { var w strings.Builder - printSnapshotGroupJSON(&w, nil, grouped) + err := printSnapshotGroupJSON(&w, nil, grouped) + rtest.OK(t, err) rtest.Equals(t, "[]", strings.TrimSpace(w.String())) } diff --git a/cmd/restic/delete.go b/cmd/restic/delete.go index e9b2481e0..26ca08196 100644 --- a/cmd/restic/delete.go +++ b/cmd/restic/delete.go @@ -9,7 +9,7 @@ import ( // DeleteFiles deletes the given fileList of fileType in parallel // it will print a warning if there is an error, but continue deleting the remaining files func DeleteFiles(gopts GlobalOptions, repo restic.Repository, fileList restic.IDSet, fileType restic.FileType) { - deleteFiles(gopts, true, repo, fileList, fileType) + _ = deleteFiles(gopts, true, repo, fileList, fileType) } // DeleteFilesChecked deletes the given fileList of fileType in parallel diff --git a/cmd/restic/exclude.go b/cmd/restic/exclude.go index e63c689ec..1ddd8932c 100644 --- a/cmd/restic/exclude.go +++ b/cmd/restic/exclude.go @@ -180,7 +180,9 @@ func isDirExcludedByFile(dir, tagFilename, header string) bool { Warnf("could not open exclusion tagfile: %v", err) return false } - defer f.Close() + defer func() { + _ = f.Close() + }() buf := make([]byte, len(header)) _, err = io.ReadFull(f, buf) // EOF is handled with a dedicated message, otherwise the warning were too cryptic diff --git a/cmd/restic/global_test.go b/cmd/restic/global_test.go index 2a0b05ceb..fee5294b5 100644 --- a/cmd/restic/global_test.go +++ b/cmd/restic/global_test.go @@ -54,7 +54,7 @@ func TestReadRepo(t *testing.T) { var opts3 GlobalOptions opts3.RepositoryFile = foo + "-invalid" - repo, err = ReadRepo(opts3) + _, err = ReadRepo(opts3) if err == nil { t.Fatal("must not read repository path from invalid file path") } diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index d17602d0b..7d198d336 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -166,7 +166,7 @@ func testRunDiffOutput(gopts GlobalOptions, firstSnapshotID string, secondSnapsh ShowMetadata: false, } err := runDiff(opts, gopts, []string{firstSnapshotID, secondSnapshotID}) - return string(buf.Bytes()), err + return buf.String(), err } func testRunRebuildIndex(t testing.TB, gopts GlobalOptions) { @@ -566,9 +566,9 @@ func TestBackupErrors(t *testing.T) { // Assume failure inaccessibleFile := filepath.Join(env.testdata, "0", "0", "9", "0") - os.Chmod(inaccessibleFile, 0000) + rtest.OK(t, os.Chmod(inaccessibleFile, 0000)) defer func() { - os.Chmod(inaccessibleFile, 0644) + rtest.OK(t, os.Chmod(inaccessibleFile, 0644)) }() opts := BackupOptions{} gopts := env.gopts @@ -657,7 +657,11 @@ func TestBackupTags(t *testing.T) { testRunBackup(t, "", []string{env.testdata}, opts, env.gopts) testRunCheck(t, env.gopts) newest, _ := testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + + if newest == nil { + t.Fatal("expected a backup, got nil") + } + rtest.Assert(t, len(newest.Tags) == 0, "expected no tags, got %v", newest.Tags) parent := newest @@ -666,7 +670,11 @@ func TestBackupTags(t *testing.T) { testRunBackup(t, "", []string{env.testdata}, opts, env.gopts) testRunCheck(t, env.gopts) newest, _ = testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + + if newest == nil { + t.Fatal("expected a backup, got nil") + } + rtest.Assert(t, len(newest.Tags) == 1 && newest.Tags[0] == "NL", "expected one NL tag, got %v", newest.Tags) // Tagged backup should have untagged backup as parent. @@ -833,7 +841,10 @@ func TestTag(t *testing.T) { testRunBackup(t, "", []string{env.testdata}, BackupOptions{}, env.gopts) testRunCheck(t, env.gopts) newest, _ := testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + if newest == nil { + t.Fatal("expected a new backup, got nil") + } + rtest.Assert(t, len(newest.Tags) == 0, "expected no tags, got %v", newest.Tags) rtest.Assert(t, newest.Original == nil, @@ -843,7 +854,9 @@ func TestTag(t *testing.T) { testRunTag(t, TagOptions{SetTags: restic.TagLists{[]string{"NL"}}}, env.gopts) testRunCheck(t, env.gopts) newest, _ = testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + if newest == nil { + t.Fatal("expected a backup, got nil") + } rtest.Assert(t, len(newest.Tags) == 1 && newest.Tags[0] == "NL", "set failed, expected one NL tag, got %v", newest.Tags) rtest.Assert(t, newest.Original != nil, "expected original snapshot id, got nil") @@ -853,7 +866,9 @@ func TestTag(t *testing.T) { testRunTag(t, TagOptions{AddTags: restic.TagLists{[]string{"CH"}}}, env.gopts) testRunCheck(t, env.gopts) newest, _ = testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + if newest == nil { + t.Fatal("expected a backup, got nil") + } rtest.Assert(t, len(newest.Tags) == 2 && newest.Tags[0] == "NL" && newest.Tags[1] == "CH", "add failed, expected CH,NL tags, got %v", newest.Tags) rtest.Assert(t, newest.Original != nil, "expected original snapshot id, got nil") @@ -863,7 +878,9 @@ func TestTag(t *testing.T) { testRunTag(t, TagOptions{RemoveTags: restic.TagLists{[]string{"NL"}}}, env.gopts) testRunCheck(t, env.gopts) newest, _ = testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + if newest == nil { + t.Fatal("expected a backup, got nil") + } rtest.Assert(t, len(newest.Tags) == 1 && newest.Tags[0] == "CH", "remove failed, expected one CH tag, got %v", newest.Tags) rtest.Assert(t, newest.Original != nil, "expected original snapshot id, got nil") @@ -874,7 +891,9 @@ func TestTag(t *testing.T) { testRunTag(t, TagOptions{RemoveTags: restic.TagLists{[]string{"CH", "US", "RU"}}}, env.gopts) testRunCheck(t, env.gopts) newest, _ = testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + if newest == nil { + t.Fatal("expected a backup, got nil") + } rtest.Assert(t, len(newest.Tags) == 0, "expected no tags, got %v", newest.Tags) rtest.Assert(t, newest.Original != nil, "expected original snapshot id, got nil") @@ -885,7 +904,9 @@ func TestTag(t *testing.T) { testRunTag(t, TagOptions{SetTags: restic.TagLists{[]string{""}}}, env.gopts) testRunCheck(t, env.gopts) newest, _ = testRunSnapshots(t, env.gopts) - rtest.Assert(t, newest != nil, "expected a new backup, got nil") + if newest == nil { + t.Fatal("expected a backup, got nil") + } rtest.Assert(t, len(newest.Tags) == 0, "expected no tags, got %v", newest.Tags) rtest.Assert(t, newest.Original != nil, "expected original snapshot id, got nil") @@ -933,7 +954,7 @@ func testRunKeyAddNewKeyUserHost(t testing.TB, gopts GlobalOptions) { keyHostname = "" }() - cmdKey.Flags().Parse([]string{"--user=john", "--host=example.com"}) + rtest.OK(t, cmdKey.Flags().Parse([]string{"--user=john", "--host=example.com"})) t.Log("adding key for john@example.com") rtest.OK(t, runKey(gopts, []string{"add"})) @@ -1106,7 +1127,7 @@ func TestRestoreLatest(t *testing.T) { testRunBackup(t, "", []string{filepath.Base(env.testdata)}, opts, env.gopts) testRunCheck(t, env.gopts) - os.Remove(p) + rtest.OK(t, os.Remove(p)) rtest.OK(t, appendRandomData(p, 101)) testRunBackup(t, "", []string{filepath.Base(env.testdata)}, opts, env.gopts) testRunCheck(t, env.gopts) @@ -1747,16 +1768,35 @@ func copyFile(dst string, src string) error { if err != nil { return err } - defer srcFile.Close() dstFile, err := os.Create(dst) if err != nil { + // ignore subsequent errors + _ = srcFile.Close() return err } - defer dstFile.Close() _, err = io.Copy(dstFile, srcFile) - return err + if err != nil { + // ignore subsequent errors + _ = srcFile.Close() + _ = dstFile.Close() + return err + } + + err = srcFile.Close() + if err != nil { + // ignore subsequent errors + _ = dstFile.Close() + return err + } + + err = dstFile.Close() + if err != nil { + return err + } + + return nil } var diffOutputRegexPatterns = []string{ diff --git a/helpers/prepare-release/main.go b/helpers/prepare-release/main.go index 70dc31afe..774397b0b 100644 --- a/helpers/prepare-release/main.go +++ b/helpers/prepare-release/main.go @@ -237,7 +237,9 @@ func preCheckChangelogVersion() { if err != nil { die("unable to open CHANGELOG.md: %v", err) } - defer f.Close() + defer func() { + _ = f.Close() + }() sc := bufio.NewScanner(f) for sc.Scan() { diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 2b0ac4ada..235898df0 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -405,7 +405,10 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous debug.Log("%v hasn't changed, but contents are missing!", target) // There are contents missing - inform user! err := errors.Errorf("parts of %v not found in the repository index; storing the file again", target) - arch.error(abstarget, fi, err) + err = arch.error(abstarget, fi, err) + if err != nil { + return FutureNode{}, false, err + } } // reopen file and do an fstat() on the open file to check it is still @@ -457,7 +460,10 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous start := time.Now() oldSubtree, err := arch.loadSubtree(ctx, previous) if err != nil { - arch.error(abstarget, fi, err) + err = arch.error(abstarget, fi, err) + } + if err != nil { + return FutureNode{}, false, err } fn.isTree = true @@ -592,7 +598,10 @@ func (arch *Archiver) SaveTree(ctx context.Context, snPath string, atree *Tree, oldNode := previous.Find(name) oldSubtree, err := arch.loadSubtree(ctx, oldNode) if err != nil { - arch.error(join(snPath, name), nil, err) + err = arch.error(join(snPath, name), nil, err) + } + if err != nil { + return nil, err } // not a leaf node, archive subtree @@ -751,7 +760,7 @@ func (arch *Archiver) loadParentTree(ctx context.Context, snapshotID restic.ID) tree, err := arch.Repo.LoadTree(ctx, *sn.Tree) if err != nil { debug.Log("unable to load tree %v: %v", *sn.Tree, err) - arch.error("/", nil, arch.wrapLoadTreeError(*sn.Tree, err)) + _ = arch.error("/", nil, arch.wrapLoadTreeError(*sn.Tree, err)) return nil } return tree diff --git a/internal/backend/backend_retry_test.go b/internal/backend/backend_retry_test.go index 9c9a6b708..a746032c7 100644 --- a/internal/backend/backend_retry_test.go +++ b/internal/backend/backend_retry_test.go @@ -62,11 +62,11 @@ func TestBackendListRetry(t *testing.T) { // fail during first retry, succeed during second retry++ if retry == 1 { - fn(restic.FileInfo{Name: ID1}) + _ = fn(restic.FileInfo{Name: ID1}) return errors.New("test list error") } - fn(restic.FileInfo{Name: ID1}) - fn(restic.FileInfo{Name: ID2}) + _ = fn(restic.FileInfo{Name: ID1}) + _ = fn(restic.FileInfo{Name: ID2}) return nil }, } diff --git a/internal/backend/foreground_test.go b/internal/backend/foreground_test.go index 34c55d1f3..81adefe32 100644 --- a/internal/backend/foreground_test.go +++ b/internal/backend/foreground_test.go @@ -23,7 +23,9 @@ func TestForeground(t *testing.T) { bg, err := backend.StartForeground(cmd) rtest.OK(t, err) - defer cmd.Wait() + defer func() { + rtest.OK(t, cmd.Wait()) + }() err = bg() rtest.OK(t, err) diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go index f1b658864..8e9625734 100644 --- a/internal/backend/local/local.go +++ b/internal/backend/local/local.go @@ -265,9 +265,15 @@ func visitDirs(ctx context.Context, dir string, fn func(restic.FileInfo) error) if err != nil { return err } - defer d.Close() sub, err := d.Readdirnames(-1) + if err != nil { + // ignore subsequent errors + _ = d.Close() + return err + } + + err = d.Close() if err != nil { return err } @@ -286,9 +292,15 @@ func visitFiles(ctx context.Context, dir string, fn func(restic.FileInfo) error) if err != nil { return err } - defer d.Close() sub, err := d.Readdir(-1) + if err != nil { + // ignore subsequent errors + _ = d.Close() + return err + } + + err = d.Close() if err != nil { return err } diff --git a/internal/backend/local/local_internal_test.go b/internal/backend/local/local_internal_test.go index a67233903..030099488 100644 --- a/internal/backend/local/local_internal_test.go +++ b/internal/backend/local/local_internal_test.go @@ -30,7 +30,9 @@ func TestNoSpacePermanent(t *testing.T) { be, err := Open(context.Background(), Config{Path: dir}) rtest.OK(t, err) - defer be.Close() + defer func() { + rtest.OK(t, be.Close()) + }() h := restic.Handle{Type: restic.ConfigFile} err = be.Save(context.Background(), h, nil) diff --git a/internal/backend/rclone/backend.go b/internal/backend/rclone/backend.go index 2f730f238..9aa36c9d3 100644 --- a/internal/backend/rclone/backend.go +++ b/internal/backend/rclone/backend.go @@ -63,9 +63,9 @@ func run(command string, args ...string) (*StdioConn, *exec.Cmd, *sync.WaitGroup stdout, w, err := os.Pipe() if err != nil { - // close first pipe - r.Close() - stdin.Close() + // close first pipe and ignore subsequent errors + _ = r.Close() + _ = stdin.Close() return nil, nil, nil, nil, err } @@ -197,8 +197,8 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) { err := cmd.Wait() debug.Log("Wait returned %v", err) be.waitResult = err - // close our side of the pipes to rclone - stdioConn.CloseAll() + // close our side of the pipes to rclone, ignore errors + _ = stdioConn.CloseAll() close(waitCh) }() @@ -228,22 +228,25 @@ func newBackend(cfg Config, lim limiter.Limiter) (*Backend, error) { // rclone is able to accept HTTP requests. url := fmt.Sprintf("http://localhost/file-%d", rand.Uint64()) - req, err := http.NewRequest(http.MethodGet, url, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, err } req.Header.Set("Accept", rest.ContentTypeV2) - req.Cancel = ctx.Done() res, err := ctxhttp.Do(ctx, client, req) if err != nil { - bg() + // ignore subsequent errors + _ = bg() _ = cmd.Process.Kill() return nil, errors.Errorf("error talking HTTP to rclone: %v", err) } debug.Log("HTTP status %q returned, moving instance to background", res.Status) - bg() + err = bg() + if err != nil { + return nil, fmt.Errorf("error moving process to background: %w", err) + } return be, nil } diff --git a/internal/backend/rclone/internal_test.go b/internal/backend/rclone/internal_test.go index b4bd01996..8bc661ab7 100644 --- a/internal/backend/rclone/internal_test.go +++ b/internal/backend/rclone/internal_test.go @@ -23,7 +23,10 @@ func TestRcloneExit(t *testing.T) { return } rtest.OK(t, err) - defer be.Close() + defer func() { + // ignore the error as the test will kill rclone (see below) + _ = be.Close() + }() err = be.cmd.Process.Kill() rtest.OK(t, err) diff --git a/internal/backend/utils.go b/internal/backend/utils.go index e6dc7a549..39c68b4ce 100644 --- a/internal/backend/utils.go +++ b/internal/backend/utils.go @@ -52,7 +52,7 @@ func DefaultLoad(ctx context.Context, h restic.Handle, length int, offset int64, } err = fn(rd) if err != nil { - rd.Close() // ignore secondary errors closing the reader + _ = rd.Close() // ignore secondary errors closing the reader return err } return rd.Close() diff --git a/internal/cache/backend.go b/internal/cache/backend.go index a58c695f7..876f30110 100644 --- a/internal/cache/backend.go +++ b/internal/cache/backend.go @@ -166,7 +166,7 @@ func (b *Backend) Load(ctx context.Context, h restic.Handle, length int, offset if err == nil { err = consumer(rd) if err != nil { - rd.Close() // ignore secondary errors + _ = rd.Close() // ignore secondary errors return err } return rd.Close() diff --git a/internal/cache/dir_test.go b/internal/cache/dir_test.go index 73a08eb4d..3076bea3d 100644 --- a/internal/cache/dir_test.go +++ b/internal/cache/dir_test.go @@ -17,7 +17,12 @@ func TestCacheDirEnv(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.Unsetenv("RESTIC_CACHE_DIR") + defer func() { + err := os.Unsetenv("RESTIC_CACHE_DIR") + if err != nil { + t.Fatal(err) + } + }() } dir, err := DefaultDir() diff --git a/internal/checker/checker.go b/internal/checker/checker.go index 38b3c75c4..e842a08be 100644 --- a/internal/checker/checker.go +++ b/internal/checker/checker.go @@ -111,7 +111,11 @@ func (c *Checker) LoadIndex(ctx context.Context) (hints []error, errs []error) { } // Merge index before computing pack sizes, as this needs removed duplicates - c.masterIndex.MergeFinalIndexes() + err = c.masterIndex.MergeFinalIndexes() + if err != nil { + // abort if an error occurs merging the indexes + return hints, append(errs, err) + } // compute pack size using index entries c.packs = c.masterIndex.PackSize(ctx, false) @@ -323,7 +327,12 @@ func (c *Checker) Structure(ctx context.Context, p *progress.Counter, errChan ch }) } - wg.Wait() + // the wait group should not return an error because no worker returns an + // error, so panic if that has changed somehow. + err := wg.Wait() + if err != nil { + panic(err) + } } func (c *Checker) checkTree(id restic.ID, tree *restic.Tree) (errs []error) { diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index ad1b15f1a..ade2813c7 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -489,7 +489,7 @@ func TestCheckerBlobTypeConfusion(t *testing.T) { Nodes: []*restic.Node{malNode, dirNode}, } - rootId, err := repo.SaveTree(ctx, rootTree) + rootID, err := repo.SaveTree(ctx, rootTree) test.OK(t, err) test.OK(t, repo.Flush(ctx)) @@ -498,12 +498,12 @@ func TestCheckerBlobTypeConfusion(t *testing.T) { snapshot, err := restic.NewSnapshot([]string{"/damaged"}, []string{"test"}, "foo", time.Now()) test.OK(t, err) - snapshot.Tree = &rootId + snapshot.Tree = &rootID - snapId, err := repo.SaveJSONUnpacked(ctx, restic.SnapshotFile, snapshot) + snapID, err := repo.SaveJSONUnpacked(ctx, restic.SnapshotFile, snapshot) test.OK(t, err) - t.Logf("saved snapshot %v", snapId.Str()) + t.Logf("saved snapshot %v", snapID.Str()) delayRepo := &delayRepository{ Repository: repo, diff --git a/internal/dump/acl.go b/internal/dump/acl.go index 9c5fd95de..48ce28a4c 100644 --- a/internal/dump/acl.go +++ b/internal/dump/acl.go @@ -101,12 +101,23 @@ func (a *acl) decode(xattr []byte) { func (a *acl) encode() []byte { buf := new(bytes.Buffer) ae := new(aclElem) - binary.Write(buf, binary.LittleEndian, &a.Version) + + err := binary.Write(buf, binary.LittleEndian, &a.Version) + // write to a bytes.Buffer always returns a nil error + if err != nil { + panic(err) + } + for _, elem := range a.List { ae.Tag = uint16(elem.getType()) ae.Perm = elem.Perm ae.ID = elem.getID() - binary.Write(buf, binary.LittleEndian, ae) + + err := binary.Write(buf, binary.LittleEndian, ae) + // write to a bytes.Buffer always returns a nil error + if err != nil { + panic(err) + } } return buf.Bytes() } diff --git a/internal/dump/common.go b/internal/dump/common.go index fd74a4a07..eb02b1c2e 100644 --- a/internal/dump/common.go +++ b/internal/dump/common.go @@ -25,7 +25,8 @@ func writeDump(ctx context.Context, repo restic.Repository, tree *restic.Tree, r rootNode.Path = rootPath err := dumpTree(ctx, repo, rootNode, rootPath, dmp) if err != nil { - dmp.Close() + // ignore subsequent errors + _ = dmp.Close() return err } diff --git a/internal/dump/zip_test.go b/internal/dump/zip_test.go index 84e0d0487..3b482244c 100644 --- a/internal/dump/zip_test.go +++ b/internal/dump/zip_test.go @@ -23,10 +23,16 @@ func readZipFile(f *zip.File) ([]byte, error) { if err != nil { return nil, err } - defer rc.Close() b := &bytes.Buffer{} _, err = b.ReadFrom(rc) + if err != nil { + // ignore subsequent errors + _ = rc.Close() + return nil, err + } + + err = rc.Close() if err != nil { return nil, err } diff --git a/internal/fs/fs_local_vss.go b/internal/fs/fs_local_vss.go index 7deef8f05..b3e08fed9 100644 --- a/internal/fs/fs_local_vss.go +++ b/internal/fs/fs_local_vss.go @@ -50,7 +50,7 @@ func (fs *LocalVss) DeleteSnapshots() { for volumeName, snapshot := range fs.snapshots { if err := snapshot.Delete(); err != nil { - fs.msgError(volumeName, errors.Errorf("failed to delete VSS snapshot: %s", err)) + _ = fs.msgError(volumeName, errors.Errorf("failed to delete VSS snapshot: %s", err)) activeSnapshots[volumeName] = snapshot } } @@ -117,7 +117,7 @@ func (fs *LocalVss) snapshotPath(path string) string { fs.msgMessage("creating VSS snapshot for [%s]\n", vssVolume) if snapshot, err := NewVssSnapshot(vssVolume, 120, fs.msgError); err != nil { - fs.msgError(vssVolume, errors.Errorf("failed to create snapshot for [%s]: %s\n", + _ = fs.msgError(vssVolume, errors.Errorf("failed to create snapshot for [%s]: %s\n", vssVolume, err)) fs.failedSnapshots[volumeNameLower] = struct{}{} } else { diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index 50128a9c0..24d8cf03d 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -219,17 +219,17 @@ func TestFuseDir(t *testing.T) { } // Test top-level directories for their UID and GID. -func TestTopUidGid(t *testing.T) { +func TestTopUIDGID(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() restic.TestCreateSnapshot(t, repo, time.Unix(1460289341, 207401672), 0, 0) - testTopUidGid(t, Config{}, repo, uint32(os.Getuid()), uint32(os.Getgid())) - testTopUidGid(t, Config{OwnerIsRoot: true}, repo, 0, 0) + testTopUIDGID(t, Config{}, repo, uint32(os.Getuid()), uint32(os.Getgid())) + testTopUIDGID(t, Config{OwnerIsRoot: true}, repo, 0, 0) } -func testTopUidGid(t *testing.T, cfg Config, repo restic.Repository, uid, gid uint32) { +func testTopUIDGID(t *testing.T, cfg Config, repo restic.Repository, uid, gid uint32) { t.Helper() ctx := context.Background() diff --git a/internal/fuse/snapshots_dir.go b/internal/fuse/snapshots_dir.go index bdbdbe60b..34484b597 100644 --- a/internal/fuse/snapshots_dir.go +++ b/internal/fuse/snapshots_dir.go @@ -229,7 +229,10 @@ func updateSnapshots(ctx context.Context, root *Root) error { if root.snCount != len(snapshots) { root.snCount = len(snapshots) - root.repo.LoadIndex(ctx) + err := root.repo.LoadIndex(ctx) + if err != nil { + return err + } root.snapshots = snapshots } root.lastCheck = time.Now() @@ -272,7 +275,10 @@ func (d *SnapshotsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { debug.Log("ReadDirAll()") // update snapshots - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update snapshot names updateSnapshotNames(d, d.root.cfg.SnapshotTemplate) @@ -314,7 +320,10 @@ func (d *SnapshotsIDSDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) debug.Log("ReadDirAll()") // update snapshots - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update snapshot ids updateSnapshotIDSNames(d) @@ -348,7 +357,10 @@ func (d *HostsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { debug.Log("ReadDirAll()") // update snapshots - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update host names updateHostsNames(d) @@ -382,7 +394,10 @@ func (d *TagsDir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) { debug.Log("ReadDirAll()") // update snapshots - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update tag names updateTagNames(d) @@ -443,7 +458,10 @@ func (d *SnapshotsDir) Lookup(ctx context.Context, name string) (fs.Node, error) sn, ok := d.names[name] if !ok { // could not find entry. Updating repository-state - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update snapshot names updateSnapshotNames(d, d.root.cfg.SnapshotTemplate) @@ -476,7 +494,10 @@ func (d *SnapshotsIDSDir) Lookup(ctx context.Context, name string) (fs.Node, err sn, ok := d.names[name] if !ok { // could not find entry. Updating repository-state - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update snapshot ids updateSnapshotIDSNames(d) @@ -499,7 +520,10 @@ func (d *HostsDir) Lookup(ctx context.Context, name string) (fs.Node, error) { _, ok := d.hosts[name] if !ok { // could not find entry. Updating repository-state - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update host names updateHostsNames(d) @@ -522,7 +546,10 @@ func (d *TagsDir) Lookup(ctx context.Context, name string) (fs.Node, error) { _, ok := d.tags[name] if !ok { // could not find entry. Updating repository-state - updateSnapshots(ctx, d.root) + err := updateSnapshots(ctx, d.root) + if err != nil { + return nil, err + } // update tag names updateTagNames(d) diff --git a/internal/hashing/writer.go b/internal/hashing/writer.go index 8eb157a9f..0b2d8c5b2 100644 --- a/internal/hashing/writer.go +++ b/internal/hashing/writer.go @@ -21,8 +21,16 @@ func NewWriter(w io.Writer, h hash.Hash) *Writer { // Write wraps the write method of the underlying writer and also hashes all data. func (h *Writer) Write(p []byte) (int, error) { + // write the data to the underlying writing n, err := h.w.Write(p) - h.h.Write(p[:n]) + + // according to the interface documentation, Write() on a hash.Hash never + // returns an error. + _, hashErr := h.h.Write(p[:n]) + if hashErr != nil { + panic(hashErr) + } + return n, err } diff --git a/internal/mock/backend.go b/internal/mock/backend.go index 930fdb3ee..e3759acbf 100644 --- a/internal/mock/backend.go +++ b/internal/mock/backend.go @@ -73,7 +73,7 @@ func (m *Backend) Load(ctx context.Context, h restic.Handle, length int, offset } err = fn(rd) if err != nil { - rd.Close() // ignore secondary errors closing the reader + _ = rd.Close() // ignore secondary errors closing the reader return err } return rd.Close() diff --git a/internal/pack/pack.go b/internal/pack/pack.go index c22f434d1..d679c658b 100644 --- a/internal/pack/pack.go +++ b/internal/pack/pack.go @@ -157,7 +157,7 @@ var ( const ( // size of the header-length field at the end of the file; it is a uint32 headerLengthSize = 4 - // constant overhead of the header independent of #entries + // HeaderSize is the header's constant overhead (independent of #entries) HeaderSize = headerLengthSize + crypto.Extension maxHeaderSize = 16 * 1024 * 1024 diff --git a/internal/pack/pack_internal_test.go b/internal/pack/pack_internal_test.go index 6502e4a35..b0a5d2862 100644 --- a/internal/pack/pack_internal_test.go +++ b/internal/pack/pack_internal_test.go @@ -61,9 +61,9 @@ func TestReadHeaderEagerLoad(t *testing.T) { expectedHeader := rtest.Random(0, entryCount*int(EntrySize)+crypto.Extension) buf := &bytes.Buffer{} - buf.Write(rtest.Random(0, dataSize)) // pack blobs data - buf.Write(expectedHeader) // pack header - binary.Write(buf, binary.LittleEndian, uint32(len(expectedHeader))) // pack header length + buf.Write(rtest.Random(0, dataSize)) // pack blobs data + buf.Write(expectedHeader) // pack header + rtest.OK(t, binary.Write(buf, binary.LittleEndian, uint32(len(expectedHeader)))) // pack header length rd := &countingReaderAt{delegate: bytes.NewReader(buf.Bytes())} @@ -104,9 +104,9 @@ func TestReadRecords(t *testing.T) { expectedHeader := totalHeader[off:] buf := &bytes.Buffer{} - buf.Write(rtest.Random(0, dataSize)) // pack blobs data - buf.Write(totalHeader) // pack header - binary.Write(buf, binary.LittleEndian, uint32(len(totalHeader))) // pack header length + buf.Write(rtest.Random(0, dataSize)) // pack blobs data + buf.Write(totalHeader) // pack header + rtest.OK(t, binary.Write(buf, binary.LittleEndian, uint32(len(totalHeader)))) // pack header length rd := bytes.NewReader(buf.Bytes()) diff --git a/internal/pack/pack_test.go b/internal/pack/pack_test.go index 471e901c3..02413247f 100644 --- a/internal/pack/pack_test.go +++ b/internal/pack/pack_test.go @@ -39,7 +39,8 @@ func newPack(t testing.TB, k *crypto.Key, lengths []int) ([]Buf, []byte, uint) { var buf bytes.Buffer p := pack.NewPacker(k, &buf) for _, b := range bufs { - p.Add(restic.TreeBlob, b.id, b.data) + _, err := p.Add(restic.TreeBlob, b.id, b.data) + rtest.OK(t, err) } _, err := p.Finalize() diff --git a/internal/repository/index_test.go b/internal/repository/index_test.go index 1084558a6..c4f0179db 100644 --- a/internal/repository/index_test.go +++ b/internal/repository/index_test.go @@ -301,7 +301,11 @@ var ( func initBenchmarkIndexJSON() { idx, _ := createRandomIndex(rand.New(rand.NewSource(0)), 200000) var buf bytes.Buffer - idx.Encode(&buf) + err := idx.Encode(&buf) + if err != nil { + panic(err) + } + benchmarkIndexJSON = buf.Bytes() } diff --git a/internal/repository/master_index.go b/internal/repository/master_index.go index f24589e3d..5aa0ceccb 100644 --- a/internal/repository/master_index.go +++ b/internal/repository/master_index.go @@ -2,6 +2,7 @@ package repository import ( "context" + "fmt" "sync" "github.com/restic/restic/internal/debug" @@ -271,7 +272,7 @@ func (mi *MasterIndex) Each(ctx context.Context) <-chan restic.PackedBlob { // Indexes that are not final are left untouched. // This merging can only be called after all index files are loaded - as // removing of superseded index contents is only possible for unmerged indexes. -func (mi *MasterIndex) MergeFinalIndexes() { +func (mi *MasterIndex) MergeFinalIndexes() error { mi.idxMutex.Lock() defer mi.idxMutex.Unlock() @@ -284,10 +285,15 @@ func (mi *MasterIndex) MergeFinalIndexes() { if !idx.Final() { newIdx = append(newIdx, idx) } else { - mi.idx[0].merge(idx) + err := mi.idx[0].merge(idx) + if err != nil { + return fmt.Errorf("MergeFinalIndexes: %w", err) + } } } mi.idx = newIdx + + return nil } const saveIndexParallelism = 4 diff --git a/internal/repository/master_index_test.go b/internal/repository/master_index_test.go index 3c279696e..f1fe9af7e 100644 --- a/internal/repository/master_index_test.go +++ b/internal/repository/master_index_test.go @@ -163,7 +163,11 @@ func TestMasterMergeFinalIndexes(t *testing.T) { finalIndexes := mIdx.FinalizeNotFinalIndexes() rtest.Equals(t, []*repository.Index{idx1, idx2}, finalIndexes) - mIdx.MergeFinalIndexes() + err := mIdx.MergeFinalIndexes() + if err != nil { + t.Fatal(err) + } + allIndexes := mIdx.All() rtest.Equals(t, 1, len(allIndexes)) @@ -191,7 +195,11 @@ func TestMasterMergeFinalIndexes(t *testing.T) { finalIndexes = mIdx.FinalizeNotFinalIndexes() rtest.Equals(t, []*repository.Index{idx3}, finalIndexes) - mIdx.MergeFinalIndexes() + err = mIdx.MergeFinalIndexes() + if err != nil { + t.Fatal(err) + } + allIndexes = mIdx.All() rtest.Equals(t, 1, len(allIndexes)) @@ -209,7 +217,7 @@ func TestMasterMergeFinalIndexes(t *testing.T) { rtest.Equals(t, 2, blobCount) } -func createRandomMasterIndex(rng *rand.Rand, num, size int) (*repository.MasterIndex, restic.BlobHandle) { +func createRandomMasterIndex(t testing.TB, rng *rand.Rand, num, size int) (*repository.MasterIndex, restic.BlobHandle) { mIdx := repository.NewMasterIndex() for i := 0; i < num-1; i++ { idx, _ := createRandomIndex(rng, size) @@ -219,7 +227,10 @@ func createRandomMasterIndex(rng *rand.Rand, num, size int) (*repository.MasterI mIdx.Insert(idx1) mIdx.FinalizeNotFinalIndexes() - mIdx.MergeFinalIndexes() + err := mIdx.MergeFinalIndexes() + if err != nil { + t.Fatal(err) + } return mIdx, lookupBh } @@ -229,12 +240,12 @@ func BenchmarkMasterIndexAlloc(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { - createRandomMasterIndex(rng, 10000, 5) + createRandomMasterIndex(b, rng, 10000, 5) } } func BenchmarkMasterIndexLookupSingleIndex(b *testing.B) { - mIdx, lookupBh := createRandomMasterIndex(rand.New(rand.NewSource(0)), 1, 200000) + mIdx, lookupBh := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 1, 200000) b.ResetTimer() @@ -244,7 +255,7 @@ func BenchmarkMasterIndexLookupSingleIndex(b *testing.B) { } func BenchmarkMasterIndexLookupMultipleIndex(b *testing.B) { - mIdx, lookupBh := createRandomMasterIndex(rand.New(rand.NewSource(0)), 100, 10000) + mIdx, lookupBh := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 100, 10000) b.ResetTimer() @@ -256,7 +267,7 @@ func BenchmarkMasterIndexLookupMultipleIndex(b *testing.B) { func BenchmarkMasterIndexLookupSingleIndexUnknown(b *testing.B) { lookupBh := restic.NewRandomBlobHandle() - mIdx, _ := createRandomMasterIndex(rand.New(rand.NewSource(0)), 1, 200000) + mIdx, _ := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 1, 200000) b.ResetTimer() @@ -267,7 +278,7 @@ func BenchmarkMasterIndexLookupSingleIndexUnknown(b *testing.B) { func BenchmarkMasterIndexLookupMultipleIndexUnknown(b *testing.B) { lookupBh := restic.NewRandomBlobHandle() - mIdx, _ := createRandomMasterIndex(rand.New(rand.NewSource(0)), 100, 10000) + mIdx, _ := createRandomMasterIndex(b, rand.New(rand.NewSource(0)), 100, 10000) b.ResetTimer() @@ -284,7 +295,7 @@ func BenchmarkMasterIndexLookupParallel(b *testing.B) { b.StopTimer() rng := rand.New(rand.NewSource(0)) - mIdx, lookupBh = createRandomMasterIndex(rng, numindices, 10000) + mIdx, lookupBh = createRandomMasterIndex(b, rng, numindices, 10000) b.StartTimer() name := fmt.Sprintf("known,indices=%d", numindices) @@ -310,7 +321,7 @@ func BenchmarkMasterIndexLookupParallel(b *testing.B) { func BenchmarkMasterIndexLookupBlobSize(b *testing.B) { rng := rand.New(rand.NewSource(0)) - mIdx, lookupBh := createRandomMasterIndex(rand.New(rng), 5, 200000) + mIdx, lookupBh := createRandomMasterIndex(b, rand.New(rng), 5, 200000) b.ResetTimer() @@ -338,7 +349,10 @@ func TestIndexSave(t *testing.T) { repo, cleanup := createFilledRepo(t, 3, 0) defer cleanup() - repo.LoadIndex(context.TODO()) + err := repo.LoadIndex(context.TODO()) + if err != nil { + t.Fatal(err) + } obsoletes, err := repo.Index().(*repository.MasterIndex).Save(context.TODO(), repo, nil, nil, nil) if err != nil { diff --git a/internal/repository/repack_test.go b/internal/repository/repack_test.go index a4505d4d2..108c167d9 100644 --- a/internal/repository/repack_test.go +++ b/internal/repository/repack_test.go @@ -201,7 +201,11 @@ func rebuildIndex(t *testing.T, repo restic.Repository) { } func reloadIndex(t *testing.T, repo restic.Repository) { - repo.SetIndex(repository.NewMasterIndex()) + err := repo.SetIndex(repository.NewMasterIndex()) + if err != nil { + t.Fatal(err) + } + if err := repo.LoadIndex(context.TODO()); err != nil { t.Fatalf("error loading new index: %v", err) } diff --git a/internal/repository/repository.go b/internal/repository/repository.go index a0d27cc3f..a13a0d784 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -419,9 +419,8 @@ func (r *Repository) saveIndex(ctx context.Context, indexes ...*Index) error { debug.Log("Saved index %d as %v", i, sid) } - r.idx.MergeFinalIndexes() - return nil + return r.idx.MergeFinalIndexes() } // SaveIndex saves all new indexes in the backend. @@ -461,7 +460,10 @@ func (r *Repository) LoadIndex(ctx context.Context) error { return errors.Fatal(err.Error()) } - r.idx.MergeFinalIndexes() + err = r.idx.MergeFinalIndexes() + if err != nil { + return err + } // remove index files from the cache which have been removed in the repo return r.PrepareCache(validIndex) @@ -781,16 +783,19 @@ func DownloadAndHash(ctx context.Context, be Loader, h restic.Handle) (tmpfile * hash = restic.IDFromHash(hrd.Sum(nil)) return ierr }) + if err != nil { - tmpfile.Close() - os.Remove(tmpfile.Name()) + // ignore subsequent errors + _ = tmpfile.Close() + _ = os.Remove(tmpfile.Name()) return nil, restic.ID{}, -1, errors.Wrap(err, "Load") } _, err = tmpfile.Seek(0, io.SeekStart) if err != nil { - tmpfile.Close() - os.Remove(tmpfile.Name()) + // ignore subsequent errors + _ = tmpfile.Close() + _ = os.Remove(tmpfile.Name()) return nil, restic.ID{}, -1, errors.Wrap(err, "Seek") } diff --git a/internal/restic/lock_unix.go b/internal/restic/lock_unix.go index 019cbbfa5..266f55580 100644 --- a/internal/restic/lock_unix.go +++ b/internal/restic/lock_unix.go @@ -38,7 +38,9 @@ func (l Lock) processExists() bool { debug.Log("error searching for process %d: %v\n", l.PID, err) return false } - defer proc.Release() + defer func() { + _ = proc.Release() + }() debug.Log("sending SIGHUP to process %d\n", l.PID) err = proc.Signal(syscall.SIGHUP) diff --git a/internal/restic/node_linux.go b/internal/restic/node_linux.go index 1e45a6ed2..bdb03651f 100644 --- a/internal/restic/node_linux.go +++ b/internal/restic/node_linux.go @@ -15,7 +15,6 @@ func (node Node) restoreSymlinkTimestamps(path string, utimes [2]syscall.Timespe if err != nil { return errors.Wrap(err, "Open") } - defer dir.Close() times := []unix.Timespec{ {Sec: utimes[0].Sec, Nsec: utimes[0].Nsec}, @@ -25,10 +24,12 @@ func (node Node) restoreSymlinkTimestamps(path string, utimes [2]syscall.Timespe err = unix.UtimesNanoAt(int(dir.Fd()), filepath.Base(path), times, unix.AT_SYMLINK_NOFOLLOW) if err != nil { + // ignore subsequent errors + _ = dir.Close() return errors.Wrap(err, "UtimesNanoAt") } - return nil + return dir.Close() } func (node Node) device() int { diff --git a/internal/restic/node_test.go b/internal/restic/node_test.go index dc2449bca..5390a90ad 100644 --- a/internal/restic/node_test.go +++ b/internal/restic/node_test.go @@ -29,7 +29,8 @@ func BenchmarkNodeFillUser(t *testing.B) { t.ResetTimer() for i := 0; i < t.N; i++ { - restic.NodeFromFileInfo(path, fi) + _, err := restic.NodeFromFileInfo(path, fi) + rtest.OK(t, err) } rtest.OK(t, tempfile.Close()) diff --git a/internal/restorer/filerestorer_test.go b/internal/restorer/filerestorer_test.go index 75433cdef..28bcd5e28 100644 --- a/internal/restorer/filerestorer_test.go +++ b/internal/restorer/filerestorer_test.go @@ -44,14 +44,6 @@ func (i *TestRepo) Lookup(bh restic.BlobHandle) []restic.PackedBlob { return packs } -func (i *TestRepo) packName(pack *packInfo) string { - return i.packsIDToName[pack.id] -} - -func (i *TestRepo) packID(name string) restic.ID { - return i.packsNameToID[name] -} - func (i *TestRepo) fileContent(file *fileInfo) string { return i.filesPathToContent[file.location] } diff --git a/internal/restorer/fileswriter.go b/internal/restorer/fileswriter.go index 1a73055c4..542a15f02 100644 --- a/internal/restorer/fileswriter.go +++ b/internal/restorer/fileswriter.go @@ -97,7 +97,8 @@ func (w *filesWriter) writeToFile(path string, blob []byte, offset int64, create _, err = wr.WriteAt(blob, offset) if err != nil { - releaseWriter(wr) + // ignore subsequent errors + _ = releaseWriter(wr) return err } diff --git a/internal/restorer/preallocate_test.go b/internal/restorer/preallocate_test.go index 05b3a8efd..fe77c183b 100644 --- a/internal/restorer/preallocate_test.go +++ b/internal/restorer/preallocate_test.go @@ -19,7 +19,9 @@ func TestPreallocate(t *testing.T) { flags := os.O_CREATE | os.O_TRUNC | os.O_WRONLY wr, err := os.OpenFile(path.Join(dirpath, "test"), flags, 0600) test.OK(t, err) - defer wr.Close() + defer func() { + test.OK(t, wr.Close()) + }() err = preallocateFile(wr, i) test.OK(t, err) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 247f2ac4d..32be0f51d 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -74,7 +74,7 @@ func saveDir(t testing.TB, repo restic.Repository, nodes map[string]Node, inode if mode == 0 { mode = 0644 } - tree.Insert(&restic.Node{ + err := tree.Insert(&restic.Node{ Type: "file", Mode: mode, ModTime: node.ModTime, @@ -86,6 +86,7 @@ func saveDir(t testing.TB, repo restic.Repository, nodes map[string]Node, inode Inode: fi, Links: lc, }) + rtest.OK(t, err) case Dir: id := saveDir(t, repo, node.Nodes, inode) @@ -94,7 +95,7 @@ func saveDir(t testing.TB, repo restic.Repository, nodes map[string]Node, inode mode = 0755 } - tree.Insert(&restic.Node{ + err := tree.Insert(&restic.Node{ Type: "dir", Mode: mode, ModTime: node.ModTime, @@ -103,6 +104,7 @@ func saveDir(t testing.TB, repo restic.Repository, nodes map[string]Node, inode GID: uint32(os.Getgid()), Subtree: &id, }) + rtest.OK(t, err) default: t.Fatalf("unknown node type %T", node) } diff --git a/internal/test/helpers.go b/internal/test/helpers.go index f01c030f3..1e127f786 100644 --- a/internal/test/helpers.go +++ b/internal/test/helpers.go @@ -95,7 +95,9 @@ func Random(seed, count int) []byte { func SetupTarTestFixture(t testing.TB, outputDir, tarFile string) { input, err := os.Open(tarFile) OK(t, err) - defer input.Close() + defer func() { + OK(t, input.Close()) + }() var rd io.Reader switch filepath.Ext(tarFile) { @@ -103,7 +105,9 @@ func SetupTarTestFixture(t testing.TB, outputDir, tarFile string) { r, err := gzip.NewReader(input) OK(t, err) - defer r.Close() + defer func() { + OK(t, r.Close()) + }() rd = r case ".bzip2": rd = bzip2.NewReader(input) diff --git a/internal/textfile/read_test.go b/internal/textfile/read_test.go index 8e8e659dc..d4de03513 100644 --- a/internal/textfile/read_test.go +++ b/internal/textfile/read_test.go @@ -25,7 +25,8 @@ func writeTempfile(t testing.TB, data []byte) (string, func()) { err = closeErr } if err != nil { - os.Remove(name) + // ignore subsequent errors + _ = os.Remove(name) t.Fatal(err) } }() diff --git a/internal/ui/json/backup.go b/internal/ui/json/backup.go index d9be31b24..b6a2ed03d 100644 --- a/internal/ui/json/backup.go +++ b/internal/ui/json/backup.go @@ -79,7 +79,10 @@ func NewBackup(term *termstatus.Terminal, verbosity uint) *Backup { func toJSONString(status interface{}) string { buf := new(bytes.Buffer) - json.NewEncoder(buf).Encode(status) + err := json.NewEncoder(buf).Encode(status) + if err != nil { + panic(err) + } return buf.String() } diff --git a/internal/ui/termstatus/background_linux_test.go b/internal/ui/termstatus/background_linux_test.go index 0880d2bc0..f5796b23b 100644 --- a/internal/ui/termstatus/background_linux_test.go +++ b/internal/ui/termstatus/background_linux_test.go @@ -12,8 +12,9 @@ func TestIsProcessBackground(t *testing.T) { if err != nil { t.Skipf("can't open terminal: %v", err) } - defer tty.Close() _, err = isProcessBackground(tty.Fd()) rtest.OK(t, err) + + _ = tty.Close() } diff --git a/internal/walker/walker_test.go b/internal/walker/walker_test.go index 0c8086956..a4e820940 100644 --- a/internal/walker/walker_test.go +++ b/internal/walker/walker_test.go @@ -28,17 +28,23 @@ func buildTreeMap(tree TestTree, m TreeMap) restic.ID { for name, item := range tree { switch elem := item.(type) { case TestFile: - res.Insert(&restic.Node{ + err := res.Insert(&restic.Node{ Name: name, Type: "file", }) + if err != nil { + panic(err) + } case TestTree: id := buildTreeMap(elem, m) - res.Insert(&restic.Node{ + err := res.Insert(&restic.Node{ Name: name, Subtree: &id, Type: "dir", }) + if err != nil { + panic(err) + } default: panic(fmt.Sprintf("invalid type %T", elem)) }