Repository: mesos Updated Branches: refs/heads/master fc2055987 -> 6553d73c2
Supported O_CLOEXEC for os::open on all platforms. Review: https://reviews.apache.org/r/26715 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6bff6eae Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6bff6eae Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6bff6eae Branch: refs/heads/master Commit: 6bff6eae6dee55d2f266f1dbcbb6f3121b5329e0 Parents: fc20559 Author: Jie Yu <[email protected]> Authored: Tue Oct 14 17:21:28 2014 -0700 Committer: Jie Yu <[email protected]> Committed: Thu Oct 16 14:58:01 2014 -0700 ---------------------------------------------------------------------- .../3rdparty/stout/include/stout/os.hpp | 85 ++++++++++++++++---- .../3rdparty/stout/tests/os_tests.cpp | 24 ++++++ 2 files changed, 93 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/6bff6eae/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp index ec41fe4..c26bd2a 100644 --- a/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp +++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp @@ -209,21 +209,15 @@ inline Try<bool> access(const std::string& path, int how) } -inline Try<int> open(const std::string& path, int oflag, mode_t mode = 0) +inline Try<Nothing> cloexec(int fd) { - int fd = ::open(path.c_str(), oflag, mode); + int flags = ::fcntl(fd, F_GETFD); - if (fd < 0) { + if (flags == -1) { return ErrnoError(); } - return fd; -} - - -inline Try<Nothing> close(int fd) -{ - if (::close(fd) != 0) { + if (::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1) { return ErrnoError(); } @@ -231,7 +225,7 @@ inline Try<Nothing> close(int fd) } -inline Try<Nothing> cloexec(int fd) +inline Try<bool> isCloexec(int fd) { int flags = ::fcntl(fd, F_GETFD); @@ -239,11 +233,7 @@ inline Try<Nothing> cloexec(int fd) return ErrnoError(); } - if (::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1) { - return ErrnoError(); - } - - return Nothing(); + return (flags & FD_CLOEXEC) != 0; } @@ -275,6 +265,69 @@ inline Try<bool> isNonblock(int fd) } +// For old systems that do not support O_CLOEXEC, we still want +// os::open to accept that flag so that we can simplify the code. +// TODO(jieyu): Move this along with nonblock/cloexec to a separate +// header 'stout/fcntl.hpp'. +#ifndef O_CLOEXEC +// Since we will define O_CLOEXEC if it is not yet defined, we use a +// special symbol to tell if the flag is truly unavailable or not. +#define O_CLOEXEC_UNDEFINED + +// NOTE: For backward compatibility concern, kernel usually does not +// change the constant values for symbols like O_CLOEXEC. +#if defined(__APPLE__) +// Copied from '/usr/include/sys/fcntl.h' +#define O_CLOEXEC 0x1000000 +#elif defined(__linux__) +// Copied from '/usr/include/asm-generic/fcntl.h'. +#define O_CLOEXEC 02000000 +#endif +#endif + + +inline Try<int> open(const std::string& path, int oflag, mode_t mode = 0) +{ +#ifdef O_CLOEXEC_UNDEFINED + // Before we passing oflag to ::open, we need to strip the O_CLOEXEC + // flag since it's not supported. + bool cloexec = false; + if ((oflag & O_CLOEXEC) != 0) { + oflag &= ~O_CLOEXEC; + cloexec = true; + } +#endif + + int fd = ::open(path.c_str(), oflag, mode); + + if (fd < 0) { + return ErrnoError(); + } + +#ifdef O_CLOEXEC_UNDEFINED + if (cloexec) { + Try<Nothing> result = os::cloexec(fd); + if (result.isError()) { + os::close(fd); + return Error("Failed to set cloexec: " + result.error()); + } + } +#endif + + return fd; +} + + +inline Try<Nothing> close(int fd) +{ + if (::close(fd) != 0) { + return ErrnoError(); + } + + return Nothing(); +} + + // Sets the access and modification times of 'path' to the current time. inline Try<Nothing> utime(const std::string& path) { http://git-wip-us.apache.org/repos/asf/mesos/blob/6bff6eae/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp index 898d175..02293f2 100644 --- a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp +++ b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp @@ -133,6 +133,30 @@ TEST_F(OsTest, system) } +TEST_F(OsTest, cloexec) +{ + Try<int> fd = os::open( + "cloexec", + O_CREAT | O_WRONLY | O_APPEND | O_CLOEXEC, + S_IRUSR | S_IWUSR | S_IRGRP | S_IRWXO); + + ASSERT_SOME(fd); + EXPECT_SOME_TRUE(os::isCloexec(fd.get())); + + close(fd.get()); + + fd = os::open( + "non-cloexec", + O_CREAT | O_WRONLY | O_APPEND, + S_IRUSR | S_IWUSR | S_IRGRP | S_IRWXO); + + ASSERT_SOME(fd); + EXPECT_SOME_FALSE(os::isCloexec(fd.get())); + + close(fd.get()); +} + + TEST_F(OsTest, nonblock) { int pipes[2];
