Isn’t what the article suggests (with defer and then WriteString) technically a race condition? Is there no way that the closer can get called before WriteString executes?
defer gets executed at the end of the current function
Don't defer Close() on writable files (2017)
141–150 of 311 posts
Re: Don't defer Close() on writable files (2017)
#142The end of the article has my favorite solution. This is also how I'd solve it in Rust, except the defer would be implicit. func doSomething() error { f, err := os.Create("foo") if err != nil { return err } defer func(){ _ = f.Close() }() if _, err := f.Write([]byte("bar"); err != nil { return err } return f.Close() }
Yes, note you’ll also see: defer f.Close() Instead of, defer func(){ _ = f.Close() }() I think this is likely a code style difference due to working with a linter that alarms on discarded error returns, but I’m not sure. Both options have the same behavior, unless you reassign f (defer f.Close() will use the original value, defer func() ... will use the current value).
Yup, I wrote it that way because I have a linter for unused error returns with a handful of exceptions (like closing http request bodies).
Re: Don't defer Close() on writable files (2017)
#143Earlier quoted context omitted.
Yes, note you’ll also see: defer f.Close() Instead of, defer func(){ _ = f.Close() }() I think this is likely a code style difference due to working with a linter that alarms on discarded error returns, but I’m not sure. Both options have the same behavior, unless you reassign f (defer f.Close() will use the original value, defer func() ... will use the current value).
Yup, I wrote it that way because I have a linter for unused error returns with a handful of exceptions (like closing http request bodies).
I think I would add an exception for this too, because it crops up a lot.
Re: Don't defer Close() on writable files (2017)
#144Earlier quoted context omitted.
https://news.ycombinator.com/item?id=26998308
Why would the comments have changed timestamps though?
The posting timestamp is temporarily adjusted, such that people don't complain about a week day old submission being on the front page.
The comment timestamps are temporarily adjusted to be consistent with the adjusted posting times, such that readers aren't confused by the comments predating the apparent posting time.
The timestamps will revert back to their original values in a few days.
What I don't understand is why this went into the second chance pool if the original submission made it to the front page and got >100 points.
Re: Don't defer Close() on writable files (2017)
#145Earlier quoted context omitted.
> I still question why defer doesn't support doing exactly that. When would it ever be useful? You'd soon start to hate life if you actually tried using the above function in anything beyond a toy application. > 99% of the time shouldn't be used 1. 99% of the time it is fine to use without further consideration. Even if there are errors, they don't matter. The example from the parent comment is a perfect case in poin…
The whole point of this post is that an error returned from file.Close DOES matter
You seem confused. The article is about writing a file where it does matter, but the comment example, which is what we're talking about, only reads a file. If close fails after read, who gives a shit? What difference is it going to make? All your read operations are complete already. Close isn't going to trigger a time machine that goes back and time and undos the reads you've performed. It is entirely inconsequential.
Re: Don't defer Close() on writable files (2017)
#146Earlier quoted context omitted.
Yup, I wrote it that way because I have a linter for unused error returns with a handful of exceptions (like closing http request bodies).
I think I would add an exception for this too, because it crops up a lot.
I can't decide because in this pattern you ignore once and check once and I like the lint for the check. Ideally the linter could recognize this pattern. Even better would be if the linter could catch when you never close, I've made that mistake a few times.
Re: Don't defer Close() on writable files (2017)
#147Isn’t what the article suggests (with defer and then WriteString) technically a race condition? Is there no way that the closer can get called before WriteString executes?
This is the way.
func doSomething() (err error) {
var f *os.File
f, err = os.Create("foo")
if err != nil {
return
}
defer func(){
if nil != f {
f.Close()
}
}()
_, err = f.Write([]byte("bar")
if err != nil {
return
}
err = f.Close()
f = nil
return
}
Re: Don't defer Close() on writable files (2017)
#148Didn't I see this thread the other day including comments? Investigating, Algolia search shows this thread as being posted 2 days ago, and the memorable comments too.
I noticed this before: https://news.ycombinator.com/item?id=40049170
Re: Don't defer Close() on writable files (2017)
#149The article suggests using a named return value `err` to allow the return value of `Close` to be be propagated - unless doing so would overwrite an earlier error:
defer func() {
cerr := f.Close()
if err == nil {
err = cerr
}
}()
defer func() {
cerr := f.Close()
err = errors.Join(err, cerr)
}()Re: Don't defer Close() on writable files (2017)
#150The article suggests using a named return value `err` to allow the return value of `Close` to be be propagated - unless doing so would overwrite an earlier error: defer func() { cerr := f.Close() if err == nil { err = cerr } }() Wouldn't it be better to use `errors.Join` in this scenario? Then if both `err` and `cerr` are non-nil, the function will return both errors (and if both are `nil`, it will return `nil`): def…
This is from 2017, errors.Join did not exist at the time. But yes, today you'd do it differently.