Is it considered to be ok to not check the return code of close(2) in base?

[Brooks Davis]

On Fri, Dec 29, 2017 at 11:48:43AM -0700, Alan Somers wrote:
> On Fri, Dec 29, 2017 at 11:27 AM, Ian Lepore <ian at freebsd.org> wrote:
> 
> > On Fri, 2017-12-29 at 10:19 -0800, Yuri wrote:
> > > Some base utilities sometimes close files that they open for their
> > > purposes without checking the error code of close(2).
> > >
> > > Is this considered to be ok, because it's just a close call and we
> > > are
> > > done with that file descriptor, or is it considered to be more
> > > appropriate to check close's error code?
> > >
> > > Maybe there is some policy that covers this?
> > >
> > > IMO, every system call's return value should be checked, just in
> > > case.
> > >
> > >
> > > Yuri
> > >
> >
> > There's really no point in checking on a close from a file opened only
> > for reading.  You can argue it should be checked on a file open for
> > writing, but often isn't because you're then confronted with the
> > question "what should/can I do if there is an error?"  If you report
> > the error and exit, then what about other files that were open at the
> > time?  They're going to be closed by the kernel as part of process
> > cleanup, with no error checking or reporting.
> >
> > Also, with the async nature of filesystems, IO errors can still happen
> > after the close, unless fsync() was used.  So if you're going to miss
> > most of the errors because of that, why bother to check at all?
> 
> I would argue the opposite.  There are very few reasons why close(s) would
> ever fail, and the most likely is EBADF.  EBADF indicates a programming
> bug, like a double close or use of an uninitialized variable.  Those could
> easily turn into worse bugs in the future.  So I think the best course of
> action is to check the return code, assert() on EBADF, and ignore, or
> possibly log, other errors.

For this specific case, I think there would be value in an option to
have the kernel kill any process that calls close(fd) where fd != -1
where EBADF would be returned.

-- Brooks
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20180105/75c85e0a/attachment.sig>