Page MenuHomePhabricator

Do not close STD* descriptors
ClosedPublic

Authored by yaron.keren on Mar 30 2017, 12:27 PM.

Details

Summary

Following r297661, disable dup workaround to disable duplicate STDOUT fd closing and instead directly prevent closing of STD* file descriptors.

Diff Detail

Repository
rL LLVM

Event Timeline

yaron.keren created this revision.Mar 30 2017, 12:27 PM
rafael accepted this revision.Mar 30 2017, 12:32 PM

LGTM

lib/Support/raw_ostream.cpp
500 ↗(On Diff #93521)

Needs a comment explaining why we don't close them and preferably also why using dup "doesn't work" (i.e., it would still have stdout open in the end).

This revision is now accepted and ready to land.Mar 30 2017, 12:32 PM
This revision was automatically updated to reflect the committed changes.
jhenderson added inline comments.Mar 31 2017, 3:35 AM
llvm/trunk/lib/Support/raw_ostream.cpp
467–469

The reason I didn't do this change previously was because of this comment, which implies that the ability to close stdout etc is desired. Of course, in neither case is stdout really closed. This comment should be modified to reflect the new behaviour.

yaron.keren marked an inline comment as done.Mar 31 2017, 5:22 AM

r299207

rnk added a subscriber: rnk.Aug 3 2017, 6:27 PM

This change left behind a great number of stale comments proclaiming the virtues of closing stdout. I think I agree, closing stdout was never worth it. Asking for tool output on stdout is asking for it to get mixed up with remark outputs, -fdump-record-layouts, and all uses of outs() in clang. We still need to clean things up.

rnk added a comment.Aug 3 2017, 6:40 PM

I updated the comments I tripped over in rL310016, but I should check the programmers manual...