[libcxx-commits] [PATCH] D59839: Open fstream files in O_CLOEXEC mode when possible.

Wed Mar 27 13:14:14 PDT 2019

danalbert added a comment.

In D59839#1444061 <https://reviews.llvm.org/D59839#1444061>, @mclow.lists wrote:

> I'm mostly worried about the change in behavior breaking existing programs.

My initial hunch was that only programs that are accidentally leaking file descriptors would be affected by this since there isn't a way to get the `FILE*` or `fd` out of an `fstream`, but I suppose you could close one of the standard streams and then create an `fstream` to have it be in a known location (or `dup`/`close` then open an `fstream` and pass that `fd` number on to the `exec`'d process). I suspect these behaviors are not all that common, but I can undo the glibc behavior change here if that would be preferred. I suspect more users would want to avoid `fd` leaks than there are users depending on being able to pass an `fstream`'s fd to an `exec`'d process, but I'm not the maintainer of that platform so I'm not really the one that should be making that decision.

In D59839#1444554 <https://reviews.llvm.org/D59839#1444554>, @ldionne wrote:

> I knew nothing about this issue so I read https://mail.gnome.org/archives/gtk-devel-list/2015-March/msg00038.html, and I'd like @danalbert to comment on it.

That article is pretty unconvincing IMO. I don't buy the argument that it's safer/easier to make sure you close all your opened file descriptors pre-exec than it is to ask the OS to track that for you whenever you open a file. The main reason the author gives ignores the existence of `F_DUPFD_CLOEXEC`.

Switching topics from whether this change is //generally// desirable, does anyone have any objections to the patch being submitted if I change `__config` so only Android gets this behavior by default? `O_CLOEXEC` is something we definitely prefer to have by default.

Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59839/new/

https://reviews.llvm.org/D59839