[libcxx] r337888 - Fix bugs in create_directory implementation.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 24 21:46:32 PDT 2018


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()




More information about the cfe-commits mailing list