diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index 474d1b403..8a16e51c0 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -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; } diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp index dfe6f20c3..f1ebaa130 100644 --- a/adb/adb_utils_test.cpp +++ b/adb/adb_utils_test.cpp @@ -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)