Author: ericwf Date: Tue Jul 24 21:46:32 2018 New Revision: 337888 URL: http://llvm.org/viewvc/llvm-project?rev=337888&view=rev Log: Fix bugs in create_directory implementation.
Libc++ was incorrectly reporting an error when the target of create_directory already exists, but was not a directory. This behavior is not specified in the most recent standard, which says no error should be reported. Additionally, libc++ failed to report an error when the attribute directory path didn't exist or didn't name a directory. This has been fixed as well. Although it's not clear if we should call status or symlink_status on the attribute directory. This patch chooses to still call status. Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directories/create_directories.pass.cpp libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/operations.cpp?rev=337888&r1=337887&r2=337888&view=diff ============================================================================== --- libcxx/trunk/src/experimental/filesystem/operations.cpp (original) +++ libcxx/trunk/src/experimental/filesystem/operations.cpp Tue Jul 24 21:46:32 2018 @@ -830,7 +830,7 @@ bool __create_directory(const path& p, e if (::mkdir(p.c_str(), static_cast<int>(perms::all)) == 0) return true; - if (errno != EEXIST || !is_directory(p)) + if (errno != EEXIST) err.report(capture_errno()); return false; } @@ -845,10 +845,12 @@ bool __create_directory(path const & p, auto st = detail::posix_stat(attributes, attr_stat, &mec); if (!status_known(st)) return err.report(mec); + if (!is_directory(st)) + return err.report(errc::not_a_directory, "the specified attribute path is invalid"); if (::mkdir(p.c_str(), attr_stat.st_mode) == 0) return true; - if (errno != EEXIST || !is_directory(p)) + if (errno != EEXIST) err.report(capture_errno()); return false; } Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directories/create_directories.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directories/create_directories.pass.cpp?rev=337888&r1=337887&r2=337888&view=diff ============================================================================== --- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directories/create_directories.pass.cpp (original) +++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directories/create_directories.pass.cpp Tue Jul 24 21:46:32 2018 @@ -66,4 +66,36 @@ TEST_CASE(create_directories_multi_level TEST_CHECK(is_directory(dir)); } +TEST_CASE(create_directory_symlinks) { + scoped_test_env env; + const path root = env.create_dir("dir"); + const path sym_dest_dead = env.make_env_path("dead"); + const path dead_sym = env.create_symlink(sym_dest_dead, "dir/sym_dir"); + const path target = env.make_env_path("dir/sym_dir/foo"); + { + std::error_code ec = GetTestEC(); + TEST_CHECK(create_directories(target, ec) == false); + TEST_CHECK(ec); + TEST_CHECK(!exists(sym_dest_dead)); + TEST_CHECK(!exists(dead_sym)); + } +} + + +TEST_CASE(create_directory_through_symlinks) { + scoped_test_env env; + const path root = env.create_dir("dir"); + const path sym_dir = env.create_symlink(root, "sym_dir"); + const path target = env.make_env_path("sym_dir/foo"); + const path resolved_target = env.make_env_path("dir/foo"); + TEST_REQUIRE(is_directory(sym_dir)); + { + std::error_code ec = GetTestEC(); + TEST_CHECK(create_directories(target, ec) == true); + TEST_CHECK(!ec); + TEST_CHECK(is_directory(target)); + TEST_CHECK(is_directory(resolved_target)); + } +} + TEST_SUITE_END() Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp?rev=337888&r1=337887&r2=337888&view=diff ============================================================================== --- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp (original) +++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory.pass.cpp Tue Jul 24 21:46:32 2018 @@ -94,9 +94,9 @@ TEST_CASE(dest_is_file) { scoped_test_env env; const path file = env.create_file("file", 42); - std::error_code ec; + std::error_code ec = GetTestEC(); TEST_CHECK(fs::create_directory(file, ec) == false); - TEST_CHECK(ec); + TEST_CHECK(!ec); TEST_CHECK(is_regular_file(file)); } Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp?rev=337888&r1=337887&r2=337888&view=diff ============================================================================== --- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp (original) +++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp Tue Jul 24 21:46:32 2018 @@ -82,9 +82,9 @@ TEST_CASE(create_directory_multi_level) const path dir = env.make_env_path("dir1/dir2"); const path dir1 = env.make_env_path("dir1"); const path attr_dir = env.create_dir("attr_dir"); - std::error_code ec; + std::error_code ec = GetTestEC(); TEST_CHECK(fs::create_directory(dir, attr_dir, ec) == false); - TEST_CHECK(ec); + TEST_CHECK(ErrorIs(ec, std::errc::no_such_file_or_directory)); TEST_CHECK(!is_directory(dir)); TEST_CHECK(!is_directory(dir1)); } @@ -94,10 +94,39 @@ TEST_CASE(dest_is_file) scoped_test_env env; const path file = env.create_file("file", 42); const path attr_dir = env.create_dir("attr_dir"); - std::error_code ec; + std::error_code ec = GetTestEC(); TEST_CHECK(fs::create_directory(file, attr_dir, ec) == false); - TEST_CHECK(ec); + TEST_CHECK(!ec); TEST_CHECK(is_regular_file(file)); } +TEST_CASE(attr_dir_is_invalid) { + scoped_test_env env; + const path file = env.create_file("file", 42); + const path dest = env.make_env_path("dir"); + const path dne = env.make_env_path("dne"); + { + std::error_code ec = GetTestEC(); + TEST_CHECK(create_directory(dest, file, ec) == false); + TEST_CHECK(ErrorIs(ec, std::errc::not_a_directory)); + } + TEST_REQUIRE(!exists(dest)); + { + std::error_code ec = GetTestEC(); + TEST_CHECK(create_directory(dest, dne, ec) == false); + TEST_CHECK(ErrorIs(ec, std::errc::not_a_directory)); + } +} + +TEST_CASE(dest_is_symlink) { + scoped_test_env env; + const path dir = env.create_dir("dir"); + const path sym = env.create_symlink("dne_sym", "dne_sym_name"); + { + std::error_code ec = GetTestEC(); + TEST_CHECK(create_directory(sym, dir, ec) == false); + TEST_CHECK(!ec); + } +} + TEST_SUITE_END() _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits