<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 27, 2019 at 1:14 PM Dan Albert via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">danalbert added a comment.<br>
<br>
In D59839#1444061 <<a href="https://reviews.llvm.org/D59839#1444061" rel="noreferrer" target="_blank">https://reviews.llvm.org/D59839#1444061</a>>, @mclow.lists wrote:<br>
<br>
> I'm mostly worried about the change in behavior breaking existing programs.<br>
<br>
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.<br>
<br>
In D59839#1444554 <<a href="https://reviews.llvm.org/D59839#1444554" rel="noreferrer" target="_blank">https://reviews.llvm.org/D59839#1444554</a>>, @ldionne wrote:<br>
<br>
> I knew nothing about this issue so I read <a href="https://mail.gnome.org/archives/gtk-devel-list/2015-March/msg00038.html" rel="noreferrer" target="_blank">https://mail.gnome.org/archives/gtk-devel-list/2015-March/msg00038.html</a>, and I'd like @danalbert to comment on it.<br>
<br>
<br>
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`.<br>
<br>
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.</blockquote><div><br></div><div>I don't have any objection if this is Android-only.</div><div>I'll take another pass over the code</div><div><br></div><div>-- Marshall</div><div><br></div></div></div>