std-aux: assert against any EBADF error in nm_close()

Previously, nm_close() accepted EBADF, as long as fd was negative.

Getting EBADF for a non-negative file descriptor is a serious bug,
because in multi threaded applications there is a race to close
an unrelated file descriptor. Also, it indicates that somebody
messed up the tracking of the resource, which indicates a bug.

Getting EBADF on a negative FD is less of a problem.
But it still indicates that the caller does something they likely
don't intend to.

Assert against any EBADF.

Note that nm_clear_fd() and nm_auto_close take already care to not
close negative "file descriptors". So, if you use those, you are
not affected.

If you want a close function that doesn't care about whether fd is set,
then maybe use `nm_clear_fd(&fd)` instead.
This commit is contained in:
Thomas Haller 2022-10-18 10:33:39 +02:00
parent ad7d5887cd
commit 3254f33b33
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728

View file

@ -1070,7 +1070,17 @@ nm_close_with_error(int fd)
int errsv = errno;
nm_assert(r == -1);
nm_assert((fd < 0 && errsv == EBADF) || (fd >= 0 && errsv != EBADF));
/* EBADF indicates a bug.
*
* - if fd is non-negative, this means the tracking of the descriptor
* got messed up. That's very bad, somebody closed a wrong FD or we
* might do so. On a multi threaded application, messing up the tracking
* of the file descriptor means we race against closing an unrelated FD.
* - if fd is negative, it may not be a bug but intentional. However, our callers
* are not supposed to call close() on a negative FD either. Assert
* against that too. */
nm_assert(errsv != EBADF);
if (errsv == EINTR) {
/* There isn't really much we can do about EINTR. On Linux, always this means