You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2016/04/10 01:48:35 UTC
[2/3] mesos git commit: Windows: Fixed bug causing `os::exists` to
report invalid paths exist.
Windows: Fixed bug causing `os::exists` to report invalid paths exist.
Currently on Windows, `os::exists` will return true if a component of a
path does not exist. For example if you have `a/fancy/path`, and you ask
`os::exists("a/fake/path")`, the result currently reports `true`. In
other words, the Windows code path only checks for the error that a file
does not exist, and ignores the error that says the path is not valid.
This commit will fix this, and also add a test that will verify we don't
regress.
Review: https://reviews.apache.org/r/45015/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/387a62e5
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/387a62e5
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/387a62e5
Branch: refs/heads/master
Commit: 387a62e57e9021c006bbe9c780fde23edc40c8d2
Parents: 36214b8
Author: Alex Clemmer <cl...@gmail.com>
Authored: Sat Apr 9 16:25:50 2016 -0700
Committer: Michael Park <mp...@apache.org>
Committed: Sat Apr 9 16:25:50 2016 -0700
----------------------------------------------------------------------
.../stout/include/stout/os/windows/exists.hpp | 17 ++++++++---
.../stout/tests/os/filesystem_tests.cpp | 32 ++++++++++++++++++++
2 files changed, 45 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/387a62e5/3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp
index 9211851..423e4a8 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp
@@ -31,15 +31,24 @@ inline bool exists(const std::string& path)
return false;
}
- // NOTE: GetFileAttributes does not support unicode natively. See also
- // "documentation"[1] for why this is a check-if-file-exists idiom.
+ // NOTE: `GetFileAttributes` redirects to either `GetFileAttributesA`
+ // (ASCII) or `GetFileAttributesW` (for `wchar`s). It returns
+ // `INVALID_FILE_ATTRIBUTES` if the file could not be opened for any reason.
+ // Checking for one of two 'not found' error codes (`ERROR_FILE_NOT_FOUND` or
+ // `ERROR_PATH_NOT_FOUND`) is a reliable test for whether the file or
+ // directory exists. See also [1] for more information on this technique.
//
// [1] http://blogs.msdn.com/b/oldnewthing/archive/2007/10/23/5612082.aspx
DWORD attributes = GetFileAttributes(absolutePath.get().c_str());
- bool fileNotFound = GetLastError() == ERROR_FILE_NOT_FOUND;
+ if (attributes == INVALID_FILE_ATTRIBUTES) {
+ DWORD error = GetLastError();
+ if (error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND) {
+ return false;
+ }
+ }
- return !((attributes == INVALID_FILE_ATTRIBUTES) && fileNotFound);
+ return true;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/387a62e5/3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp b/3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp
index c93b827..6262892 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp
@@ -135,6 +135,38 @@ TEST_F(FsTest, Mkdir)
}
+TEST_F(FsTest, Exists)
+{
+ const hashset<string> EMPTY;
+ const string tmpdir = os::getcwd();
+
+ hashset<string> expectedListing = EMPTY;
+ ASSERT_EQ(expectedListing, listfiles(tmpdir));
+
+ // Create simple directory structure.
+ ASSERT_SOME(os::mkdir(path::join(tmpdir, "a", "b", "c")));
+
+ // Expect all the directories exist.
+ EXPECT_TRUE(os::exists(tmpdir));
+ EXPECT_TRUE(os::exists(path::join(tmpdir, "a")));
+ EXPECT_TRUE(os::exists(path::join(tmpdir, "a", "b")));
+ EXPECT_TRUE(os::exists(path::join(tmpdir, "a", "b", "c")));
+
+ // Return false if a component of the path does not exist.
+ EXPECT_FALSE(os::exists(path::join(tmpdir, "a", "fakeDir")));
+ EXPECT_FALSE(os::exists(path::join(tmpdir, "a", "fakeDir", "c")));
+
+ // Add file to directory tree.
+ ASSERT_SOME(os::touch(path::join(tmpdir, "a", "b", "c", "yourFile")));
+
+ // Assert it exists.
+ EXPECT_TRUE(os::exists(path::join(tmpdir, "a", "b", "c", "yourFile")));
+
+ // Return false if file is wrong.
+ EXPECT_FALSE(os::exists(path::join(tmpdir, "a", "b", "c", "yourFakeFile")));
+}
+
+
TEST_F(FsTest, Touch)
{
const string testfile = path::join(os::getcwd(), UUID::random().toString());