3

Another question How to read/write from/to file using Go? got into safe closing of file descriptors in a comment.

Note that these examples aren't checking the error return from fo.Close(). From the Linux man pages close(2): Not checking the return value of close() is a common but nevertheless serious programming error. It is quite possible that errors on a previous write(2) operation are first reported at the final close(). Not checking the return value when closing the file may lead to silent loss of data. This can especially be observed with NFS and with disk quota. – Nick Craig-Wood Jan 25 '13 at 7:12

The solution that updated the post used a panic:

// close fo on exit and check for its returned error
defer func() {
    if err := fo.Close(); err != nil {
        panic(err)
    }
}()

I want to hand this error as a value instead of panicking.

Brian
  • 987
  • 10
  • 19

1 Answers1

5

If we are afraid of writes not being completed close isn't enough, so updating the error is still not correct.

The correct solution if you want to not hit this is to fsync the file(s):

defer(fd.Close())
// Do stuff
return fd.Sync()

It's easier to read then returning a non-nil modified error either through defer or maintaining throughout the function.

This will be a performance hit, but will catch both close errors for writing to buffers and the physical write to disk.

Brian
  • 987
  • 10
  • 19
  • 1
    «This means that the anonymous function has to be defined in each function» is a false claim. Say, you can declare `func checkedClose(c io.Closer, perr *error)` and schedule its deferred call using `defer checkedClose(fd, &err)`. IOW, this is one of those rare cases where a pointer to an interface value comes in handy. – kostix Nov 09 '20 at 10:05
  • One more thing to consider is that you appear to have fixated on the mechanics of handling the errors in deferred calls which release external resources such as files, while instead the mechanics themselves are of secondary importance. Observe that in your solution you have bascally replaced silencing of an error when closing a file with writing a message to the standard error stream (or whatever the `log`'s output is redirected, if done). While this might be marginally better than completely ignoring the error, still, you will have hard time adapting your generic solution to perform… – kostix Nov 09 '20 at 10:09
  • …useful actions on a failed call to close. There are multiple things to consider: was file opened for reading only? If yes, and reading happened without errors, failure to close the file might be ignored (a warning may be logged but otherwise you're safe). Was file opened for writing? If yes, a failure to close it might indeed be grave. If so, a mere warning might not be enough: you might want to _mark_ the whole operation, of which writing this file was a part, as failed, or you might want to interactively allow the user to retry saving or maybe something else completely. – kostix Nov 09 '20 at 10:12
  • @kostix Great point about the interface reference! I'll definitely make that change. As for the example code it's just showing the functionality. In my actual use case a file is opened with truncate and write and then I use `os.Copy` to read a temporary files content and write to the fd. I don't have a retry functionality rn, but that may be the right solution if the fn returns a close error. – Brian Nov 09 '20 at 19:18
  • Please consider not using the word "reference" in most cases when talking about Go ;-) Go does not have references; it has multiple built-in types which can be said to _have reference semantics_ when their values are passed around (these types are slices, strings, maps, channels, functions and pointers), and any user-defined `struct` type which includes a field of type which has reference semantics automatically "inherits" these. But still, Go does not have references (for instance, C++ and C# have them as a language concept, but not Go), it has pointers, and I suggested to use a pointer ;-) – kostix Nov 22 '20 at 13:02