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

[Marshall Clow via libcxx-commits]

On Wed, Mar 27, 2019 at 1:14 PM Dan Albert via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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.


I don't have any objection if this is Android-only.
I'll take another pass over the code

-- Marshall
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190327/60ab4d5d/attachment.html>