Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WriteFile ignores Close() error #7

Open
kevinburke opened this issue Nov 27, 2017 · 3 comments
Open

WriteFile ignores Close() error #7

kevinburke opened this issue Nov 27, 2017 · 3 comments

Comments

@kevinburke
Copy link

Maybe it's fine, but defer f.Close() can end up ignoring an error if f.Close() returns one. Maybe this is fine since we're already returning an error if f.Close() returns a non-nil err and it would be hard to return two errors at once.

@dchest
Copy link
Owner

dchest commented Nov 27, 2017

Yes, it's fine, Close can be safely called without error checking if you do Commit call and check its error. WriteFile returns the result of Commit.

See this comment on Close function:

// If the file has been committed, Close is no-op.

https://github.com/dchest/safefile/blob/master/safefile.go#L102

(If the file was not committed, you'll get the error from Commit).

Thanks for code review anyway!

@dchest dchest closed this as completed Nov 27, 2017
@dchest
Copy link
Owner

dchest commented Nov 27, 2017

Actually, you're right: we won't detect a failure to remove file if renaming fails. https://github.com/dchest/safefile/blob/master/safefile.go#L124

@dchest dchest reopened this Nov 27, 2017
@dchest
Copy link
Owner

dchest commented Nov 27, 2017

... ah, but in this case, we still get the actual error indicating that rename failed. So, there will be an error anyway 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants