You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2017/12/19 20:18:36 UTC
[1/2] mesos git commit: Windows: Removed manual use of `O_BINARY`.
Repository: mesos
Updated Branches:
refs/heads/master 0cc636b2d -> 45669854d
Windows: Removed manual use of `O_BINARY`.
This is now superfluous as it is added in stout.
Review: https://reviews.apache.org/r/64690
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/45669854
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/45669854
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/45669854
Branch: refs/heads/master
Commit: 45669854d27c902e6bbd2d9845c1dbde74620380
Parents: 13d67ef
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Mon Dec 18 13:59:38 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue Dec 19 11:16:58 2017 -0800
----------------------------------------------------------------------
src/status_update_manager/status_update_manager_process.hpp | 3 ---
1 file changed, 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/45669854/src/status_update_manager/status_update_manager_process.hpp
----------------------------------------------------------------------
diff --git a/src/status_update_manager/status_update_manager_process.hpp b/src/status_update_manager/status_update_manager_process.hpp
index 5dc2bec..4a23573 100644
--- a/src/status_update_manager/status_update_manager_process.hpp
+++ b/src/status_update_manager/status_update_manager_process.hpp
@@ -634,9 +634,6 @@ private:
// Open the updates file.
Try<int_fd> result = os::open(
path.get(),
-#ifdef __WINDOWS__
- O_BINARY |
-#endif // __WINDOWS__
O_CREAT | O_SYNC | O_WRONLY | O_CLOEXEC,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
[2/2] mesos git commit: Windows: Fixed `os::open()` to always use
`O_BINARY`.
Posted by an...@apache.org.
Windows: Fixed `os::open()` to always use `O_BINARY`.
Previously, we had been manually adding the `O_BINARY` flag as we
encountered bugs due to the Windows default behavior of performing
line-ending translation. This was error prone.
Given this precedent, it seems safe to assume that all our existing code
expects the POSIX semantics of "binary mode", that is, no translation of
written data at all. So now we add this flag by default in `os::open()`
instead of in the users.
It is possible that a future use requires text translation. At such
point, we can trivially fix `os::open()` to take a boolean flag to
control the addition of `O_BINARY`, but we do not currently need to
engineer this.
Review: https://reviews.apache.org/r/64689
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/13d67efa
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/13d67efa
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/13d67efa
Branch: refs/heads/master
Commit: 13d67efa006d1a84822251759ae08781ea0bb7bc
Parents: 0cc636b
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Mon Dec 18 13:55:40 2017 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Tue Dec 19 11:16:58 2017 -0800
----------------------------------------------------------------------
3rdparty/stout/include/stout/net.hpp | 3 ++-
3rdparty/stout/include/stout/os/open.hpp | 5 ++++-
3rdparty/stout/include/stout/os/write.hpp | 6 ------
3rdparty/stout/include/stout/protobuf.hpp | 27 +++-----------------------
4 files changed, 9 insertions(+), 32 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/13d67efa/3rdparty/stout/include/stout/net.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/net.hpp b/3rdparty/stout/include/stout/net.hpp
index 7b6557d..abb0144 100644
--- a/3rdparty/stout/include/stout/net.hpp
+++ b/3rdparty/stout/include/stout/net.hpp
@@ -164,7 +164,8 @@ inline Try<int> download(const std::string& url, const std::string& path)
// We don't bother introducing a `os::fdopen` since this is the only place
// we use `fdopen` in the entire codebase as of writing this comment.
#ifdef __WINDOWS__
- FILE* file = ::_fdopen(fd->crt(), "w");
+ // We open in "binary" mode on Windows to avoid line-ending translation.
+ FILE* file = ::_fdopen(fd->crt(), "wb");
#else
FILE* file = ::fdopen(fd.get(), "w");
#endif
http://git-wip-us.apache.org/repos/asf/mesos/blob/13d67efa/3rdparty/stout/include/stout/os/open.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/open.hpp b/3rdparty/stout/include/stout/os/open.hpp
index 1443b63..6711f22 100644
--- a/3rdparty/stout/include/stout/os/open.hpp
+++ b/3rdparty/stout/include/stout/os/open.hpp
@@ -40,7 +40,10 @@ inline Try<int_fd> open(const std::string& path, int oflag, mode_t mode = 0)
{
#ifdef __WINDOWS__
std::wstring longpath = ::internal::windows::longpath(path);
- int_fd fd = ::_wopen(longpath.data(), oflag, mode);
+ // By default, Windows will perform "text translation" meaning that it will
+ // automatically write CR/LF instead of LF line feeds. To prevent this, and
+ // use the POSIX semantics, we open with `O_BINARY`.
+ int_fd fd = ::_wopen(longpath.data(), oflag | O_BINARY, mode);
#else
int_fd fd = ::open(path.c_str(), oflag, mode);
#endif // __WINDOWS__
http://git-wip-us.apache.org/repos/asf/mesos/blob/13d67efa/3rdparty/stout/include/stout/os/write.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/write.hpp b/3rdparty/stout/include/stout/os/write.hpp
index 4c718b1..9ff749f 100644
--- a/3rdparty/stout/include/stout/os/write.hpp
+++ b/3rdparty/stout/include/stout/os/write.hpp
@@ -108,13 +108,7 @@ inline Try<Nothing> write(const std::string& path, const std::string& message)
{
Try<int_fd> fd = os::open(
path,
-#ifdef __WINDOWS__
- // NOTE: The `O_BINARY` option turns off automatic translation
- // of newline characters to Windows-style line endings.
- O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_BINARY,
-#else
O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC,
-#endif // __WINDOWS__
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
if (fd.isError()) {
http://git-wip-us.apache.org/repos/asf/mesos/blob/13d67efa/3rdparty/stout/include/stout/protobuf.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/protobuf.hpp b/3rdparty/stout/include/stout/protobuf.hpp
index baad126..f00a048 100644
--- a/3rdparty/stout/include/stout/protobuf.hpp
+++ b/3rdparty/stout/include/stout/protobuf.hpp
@@ -112,16 +112,9 @@ Try<Nothing> write(
template <typename T>
Try<Nothing> write(const std::string& path, const T& t)
{
- int operation_flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC;
-#ifdef __WINDOWS__
- // NOTE: Windows does automatic linefeed conversions in I/O on text files.
- // We include the `_O_BINARY` flag here to avoid this.
- operation_flags |= _O_BINARY;
-#endif // __WINDOWS__
-
Try<int_fd> fd = os::open(
path,
- operation_flags,
+ O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
if (fd.isError()) {
@@ -143,16 +136,9 @@ inline Try<Nothing> append(
const std::string& path,
const google::protobuf::Message& message)
{
- int operation_flags = O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC;
-#ifdef __WINDOWS__
- // NOTE: Windows does automatic linefeed conversions in I/O on text files.
- // We include the `_O_BINARY` flag here to avoid this.
- operation_flags |= _O_BINARY;
-#endif // __WINDOWS__
-
Try<int_fd> fd = os::open(
path,
- operation_flags,
+ O_WRONLY | O_CREAT | O_APPEND | O_CLOEXEC,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
if (fd.isError()) {
@@ -356,16 +342,9 @@ Result<T> read(int_fd fd, bool ignorePartial = false, bool undoFailed = false)
template <typename T>
Result<T> read(const std::string& path)
{
- int operation_flags = O_RDONLY | O_CLOEXEC;
-#ifdef __WINDOWS__
- // NOTE: Windows does automatic linefeed conversions in I/O on text files.
- // We include the `_O_BINARY` flag here to avoid this.
- operation_flags |= _O_BINARY;
-#endif // __WINDOWS__
-
Try<int_fd> fd = os::open(
path,
- operation_flags,
+ O_RDONLY | O_CLOEXEC,
S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
if (fd.isError()) {