adb: mkdirs fixes

Fix pathological case where the directory to be created can't be created
because there is already a file there. This was previously returning
success because the wrong var was passed to directory_exists().

Fix test to exercise this situation. Also clarify tests.

Change-Id: I0dc0f14084e0eda4e1498874d4ab2a6445d322ac
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>
(cherry picked from commit 85c45bd5a1)
This commit is contained in:
Spencer Low 2016-02-10 15:03:50 -08:00 committed by Josh Gao
parent d302a15a60
commit a1071c6924
2 changed files with 18 additions and 12 deletions

View file

@ -134,7 +134,7 @@ std::string adb_dirname(const std::string& path) {
return result;
}
// Given a relative or absolute filepath, create the parent directory hierarchy
// Given a relative or absolute filepath, create the directory hierarchy
// as needed. Returns true if the hierarchy is/was setup.
bool mkdirs(const std::string& path) {
// TODO: all the callers do unlink && mkdirs && adb_creat ---
@ -157,12 +157,12 @@ bool mkdirs(const std::string& path) {
return true;
}
const std::string parent(adb_dirname(path));
// If dirname returned the same path as what we passed in, don't go recursive.
// This can happen on Windows when walking up the directory hierarchy and not
// finding anything that already exists (unlike POSIX that will eventually
// find . or /).
const std::string parent(adb_dirname(path));
if (parent == path) {
errno = ENOENT;
return false;
@ -174,14 +174,14 @@ bool mkdirs(const std::string& path) {
}
// Now that the parent directory hierarchy of 'path' has been ensured,
// create parent itself.
// create path itself.
if (adb_mkdir(path, 0775) == -1) {
// Can't just check for errno == EEXIST because it might be a file that
// exists.
const int saved_errno = errno;
if (directory_exists(parent)) {
// If someone else created the directory, that is ok.
if (directory_exists(path)) {
return true;
}
// There might be a pre-existing file at 'path', or there might have been some other error.
errno = saved_errno;
return false;
}

View file

@ -112,20 +112,26 @@ TEST(adb_utils, adb_dirname) {
}
void test_mkdirs(const std::string basepath) {
EXPECT_TRUE(mkdirs(adb_dirname(basepath)));
EXPECT_NE(-1, adb_creat(basepath.c_str(), 0600));
EXPECT_FALSE(mkdirs(basepath + "/subdir/"));
// Test creating a directory hierarchy.
EXPECT_TRUE(mkdirs(basepath));
// Test finding an existing directory hierarchy.
EXPECT_TRUE(mkdirs(basepath));
const std::string filepath = basepath + "/file";
// Verify that the hierarchy was created by trying to create a file in it.
EXPECT_NE(-1, adb_creat(filepath.c_str(), 0600));
// If a file exists where we want a directory, the operation should fail.
EXPECT_FALSE(mkdirs(filepath));
}
TEST(adb_utils, mkdirs) {
TemporaryDir td;
// Absolute paths.
test_mkdirs(std::string(td.path) + "/dir/subdir/file");
test_mkdirs(std::string(td.path) + "/dir/subdir");
// Relative paths.
ASSERT_EQ(0, chdir(td.path)) << strerror(errno);
test_mkdirs(std::string("relative/subrel/file"));
test_mkdirs(std::string("relative/subrel"));
}
#if !defined(_WIN32)