Skip to content

[th/nm-close] all: cleanup close() handling and clarify nm_close()/nm_close_real()

Thomas Haller requested to merge th/nm-close into main

Cleanup the handling of close().

First of all, closing an invalid file descriptor (EBADF) is always a serious bug. We want to catch that. Hence, we should use nm_close() (or nm_close_real()) which asserts against such bugs. Don't ever use close() directly, to get that additional assertion.

Second, as man close comments ("Dealing with error returns from close()"), there isn't really much that can be done about errors. Especially on Linux, fd is always closed afterwards. That is even the case for EINTR, despite some POSIX systems expect to retry on EINTR or even leave the FD either open or closed. On Linux, it is always closed and it is wrong to retry. Hence, it is usually wrong to care about the error code of close() (except EBADF, which we handle by asserting). Adjust nm_close() to reflect that. The function now returns no more error and preserves the caller's errno.

EDIT: in some cases it is required to check for errors. I updated the branch to reflect that.

nm_close() is now the safe and correct way to use it. And its API makes it clear, that you are not supposed to handle an error.

Also add nm_close_real(), for the unusual case where you care about the error. Again, there isn't much you can do about the error, but maybe you want to log about it. In that case, use nm_close_real() and log a warning.

TL;DR: use almost always nm_close(). Unless you want to log the error code, then use nm_close_real(). Never use close() directly.

There is much reading on the internet about handling errors of close and in particular EINTR. See the following links:

Edited by Thomas Haller

Merge request reports