This is an archive of the discontinued LLVM Phabricator instance.

[Support] Use O_CLOEXEC only when declared
ClosedPublic

Authored by labath on Jan 19 2017, 1:33 AM.

Details

Summary

Use the O_CLOEXEC flag only when it is available. Some old systems (e.g.
SLES10) do not support this flag. POSIX explicitly guarantees that this
flag can be checked for using #if, so there is no need for a CMake
check.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Jan 19 2017, 1:33 AM
labath edited edge metadata.Jan 19 2017, 1:56 AM

In the original version my patch (D28854) I had fallback code which sets the FD_CLOEXEC flag using fcntl(2). What do you think about using that? It's a bit racy, but it's better than nothing.

I intend to start using these function from lldb, so if anyone will use lldb on the old systems, he'll get descriptor leakage there.

Yeah, I think that'll work for me.

labath commandeered this revision.Jan 23 2017, 3:06 AM
labath edited reviewers, added: mgorny; removed: labath.

Yeah, I think that'll work for me.

Now I am not sure whether that meant "yeah, that'll work and i'll do it", or "yeah, go ahead and you do it" :P, but based on lack of activity, it seems to be latter.

I'll go ahead and add those then. I'd very much appreciate if you could test it for me though.

labath updated this revision to Diff 85338.Jan 23 2017, 3:16 AM
labath edited the summary of this revision. (Show Details)

Add FD_CLOEXEC fallback.

mgorny edited edge metadata.Jan 23 2017, 4:04 AM

Haha, I thought you meant you'll do it since you had the code already :-)

mgorny accepted this revision.Jan 24 2017, 1:42 AM

Builds fine for me.

This revision is now accepted and ready to land.Jan 24 2017, 1:42 AM

@rafael: I am going to assume you are fine with this as well. Let me know if you have concerns.

This revision was automatically updated to reflect the committed changes.