Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Dec 2017 11:48:43 -0700
From:      Alan Somers <asomers@freebsd.org>
To:        Ian Lepore <ian@freebsd.org>
Cc:        Yuri <yuri@rawbw.com>, Freebsd hackers list <freebsd-hackers@freebsd.org>
Subject:   Re: Is it considered to be ok to not check the return code of close(2) in base?
Message-ID:  <CAOtMX2jSonHQ9xzVD3Q9XS2twBm_CT3Tquwn%2Bf6zmc7aV0QerQ@mail.gmail.com>
In-Reply-To: <1514572041.12000.7.camel@freebsd.org>
References:  <24acbd94-c52f-e71a-8a96-d608a10963c6@rawbw.com> <1514572041.12000.7.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Dec 29, 2017 at 11:27 AM, Ian Lepore <ian@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?
>
> -- Ian
>

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.

-Alan



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOtMX2jSonHQ9xzVD3Q9XS2twBm_CT3Tquwn%2Bf6zmc7aV0QerQ>