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

Mark Millard markmi at dsl-only.net
Mon Jan 1 05:03:18 UTC 2018


On 2017-Dec-31, at 7:17 PM, Mark Millard <markmi at dsl-only.net> wrote:


> On 2017-Dec-31, at 7:05 PM, Rodney W. Grimes <freebsd-rwg at pdx.rh.CN85.dnsmgr.net> wrote:
> 
>>> Poul-Henning Kamp phk at phk.freebsd.dk wrote on
>>> Sat Dec 30 20:35:35 UTC 2017 :
>>> 
>>>> But if you just close a file, and you're 100% sure that will work,
>>>> you should write it as:
>>>> 
>>>> 	assert(close(fd) == 0);
>>>> 
>>>> To tell the rest of us about your assumption and your confidence in it.
>>> 
>>> Quoting the FreeBSD assert man page:
>>> 
>>> QUOTE
>>>    The assert() macro	may be removed at compile time by defining NDEBUG as a
>>>    macro (e.g., by using the cc(1) option -DNDEBUG).
>>> ENDQUOTE
>>> 
>>> This makes required-actions inside asserts dangerous,
>>> at least without guarantees that NDEBUG will be
>>> undefined. Trying to guarantee that NDEBUG will be
>>> undefined would generally be a bad idea.
>>> 
>>> So,
>>> 
>>> assert(close(fd) == 0);
>>> 
>>> is a bad coding practice in my view.
>> 
>> Less bad than either of
>> 	(void)close(fd));
>> 	close(fd);
> 
> Hmm. If one is not to check the status code,
> then just do not close the file, at least for
> NDEBUG style compiles. Seems odd to me.
> 
> "assert" indicates optional code, not required
> code. (This is despite its name.)
> 
> In my view: Both errors need to be avoided and
> my point still stands: the example is not a
> reasonable way to code.
> 
> One could invent an alternate to assert under
> a related name that does not make code
> disappear (not even for NDEBUG). "assert"
> is just not the right facility to express
> just:
> 
> tell the rest of us about your assumption and your confidence in it
> 
> without also indicating: optional code.

Going in another direction for when NDEBUG
is undefined:

assert(expression_evaluating_to_false) is,
in part, defined to call abort(). abort()
in turn is defined such that:

"functions passed to atexit() are not called"

also, "[w]hether open resources such as files are
closed is implementation defined". (Quoted from
http://en.cppreference.com/w/c/program/abort .
I can get the standard's wording if needed.)

Open Group Base Specifications Issue 7 states
in its description of abort: "The functionality
described on this reference page is aligned with
the ISO C standard. Any conflict between the
requirements described here and the ISO C
standard is unintentional. This volume of
POSIX.1-2008 defers to the ISO C standard".
Before Issue 6 the wording apparently required
an attempt to fclose all streams --but that
made abort necessarily not async-signal-safe.
C99 requires the async-signal-safe status.
( http://pubs.opengroup.org/onlinepubs/9699919799/functions/abort.html )

As for C++: "Destructors of variables with
automatic, thread local (since C++11) and
static storage durations are not called.
Functions registered with std::atexit() and
std::at_quick_exit (since C++11) are also
not called. Whether open resources such as
files are closed is implementation defined."
( http://en.cppreference.com/w/cpp/utility/program/abort )

asserts that call abort are difficult to
guarantee specific program-exit behavior
for, based on just the standards anyway.

===
Mark Millard
markmi at dsl-only.net




More information about the freebsd-hackers mailing list