PR libstdc++/86910 fix filesystem::create_directories

Implement the proposed semantics from P1164R0, which reverts the changes
of LWG 2935. This means that failure to create a directory because a
non-directory already exists with that name will be reported as an
error.

While rewriting the function, also fix PR 87846, which is a result of
the C++17 changes to how a trailing slash on a path affects the last
component of a path.

	PR libstdc++/86910
	PR libstdc++/87846
	* src/filesystem/ops.cc (experimental::create_directories): Report
	an error when the path resolves to an existing non-directory (P1164).
	* src/filesystem/std-ops.cc (create_directories): Likewise. Handle
	empty filenames due to trailing slashes.
	* testsuite/27_io/filesystem/operations/create_directories.cc: Test
	when some component of the path exists and is not a directory. Test
	trailing slashes.
	* testsuite/experimental/filesystem/operations/create_directories.cc:
	Likewise.

From-SVN: r266598

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 1107da0f53e85d7a289ecd68ca3518433ec0f286..607edb9beb31eecd5023e98134205fd056be966a 100644 (file)
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,17 @@
+2018-11-29  Jonathan Wakely  <jwakely@redhat.com>
+
+       PR libstdc++/86910
+       PR libstdc++/87846
+       * src/filesystem/ops.cc (experimental::create_directories): Report
+       an error when the path resolves to an existing non-directory (P1164).
+       * src/filesystem/std-ops.cc (create_directories): Likewise. Handle
+       empty filenames due to trailing slashes.
+       * testsuite/27_io/filesystem/operations/create_directories.cc: Test
+       when some component of the path exists and is not a directory. Test
+       trailing slashes.
+       * testsuite/experimental/filesystem/operations/create_directories.cc:
+       Likewise.
+
 2018-11-28  Jonathan Wakely  <jwakely@redhat.com>
 
        PR libstdc++/83306

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 40cadbf627014a9a42adfb1dbdec7bd263496394..a6ae75ff7346a112b8986dba816144c82b27e851 100644 (file)
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -423,6 +423,19 @@ fs::create_directories(const path& p, error_code& ec) noexcept
       ec = std::make_error_code(errc::invalid_argument);
       return false;
     }
+
+  file_status st = symlink_status(p, ec);
+  if (is_directory(st))
+    return false;
+  else if (ec && !status_known(st))
+    return false;
+  else if (exists(st))
+    {
+      if (!ec)
+       ec = std::make_error_code(std::errc::not_a_directory);
+      return false;
+    }
+
   std::stack<path> missing;
   path pp = p;
 
@@ -431,24 +444,29 @@ fs::create_directories(const path& p, error_code& ec) noexcept
       ec.clear();
       const auto& filename = pp.filename();
       if (!is_dot(filename) && !is_dotdot(filename))
-       missing.push(pp);
-      pp.remove_filename();
+       {
+         missing.push(std::move(pp));
+         pp = missing.top().parent_path();
+       }
+      else
+       pp = pp.parent_path();
     }
 
   if (ec || missing.empty())
     return false;
 
+  bool created;
   do
     {
       const path& top = missing.top();
-      create_directory(top, ec);
-      if (ec && is_directory(top))
-       ec.clear();
+      created = create_directory(top, ec);
+      if (ec)
+       return false;
       missing.pop();
     }
-  while (!missing.empty() && !ec);
+  while (!missing.empty());
 
-  return missing.empty();
+  return created;
 }
 
 namespace

diff --git a/libstdc++-v3/src/filesystem/std-ops.cc b/libstdc++-v3/src/filesystem/std-ops.cc
index e266fa6d3f8701a68219053da4827cd53f58cf8d..2f9a76ffaa1688a30b141d72f99cac210375fda1 100644 (file)
--- a/libstdc++-v3/src/filesystem/std-ops.cc
+++ b/libstdc++-v3/src/filesystem/std-ops.cc
@@ -646,38 +646,74 @@ fs::create_directories(const path& p, error_code& ec)
       ec = std::make_error_code(errc::invalid_argument);
       return false;
     }
+
+  file_status st = symlink_status(p, ec);
+  if (is_directory(st))
+    return false;
+  else if (ec && !status_known(st))
+    return false;
+  else if (exists(st))
+    {
+      if (!ec)
+       ec = std::make_error_code(std::errc::not_a_directory);
+      return false;
+    }
+
   std::stack<path> missing;
   path pp = p;
 
-  while (pp.has_filename() && status(pp, ec).type() == file_type::not_found)
+  // Strip any trailing slash
+  if (pp.has_relative_path() && !pp.has_filename())
+    pp = pp.parent_path();
+
+  do
     {
-      ec.clear();
       const auto& filename = pp.filename();
-      if (!is_dot(filename) && !is_dotdot(filename))
-       missing.push(pp);
-      pp = pp.parent_path();
+      if (is_dot(filename) || is_dotdot(filename))
+       pp = pp.parent_path();
+      else
+       {
+         missing.push(std::move(pp));
+         if (missing.size() > 1000) // sanity check
+           {
+             ec = std::make_error_code(std::errc::filename_too_long);
+             return false;
+           }
+         pp = missing.top().parent_path();
+       }
 
-      if (missing.size() > 1000) // sanity check
+      if (pp.empty())
+       break;
+
+      st = status(pp, ec);
+      if (exists(st))
        {
-         ec = std::make_error_code(std::errc::filename_too_long);
-         return false;
+         if (ec)
+           return false;
+         if (!is_directory(st))
+           {
+             ec = std::make_error_code(std::errc::not_a_directory);
+             return false;
+           }
        }
-    }
 
-  if (ec || missing.empty())
-    return false;
+      if (ec && exists(st))
+       return false;
+    }
+  while (st.type() == file_type::not_found);
 
+  bool created;
   do
     {
       const path& top = missing.top();
-      create_directory(top, ec);
-      if (ec && is_directory(top))
-       ec.clear();
+      created = create_directory(top, ec);
+      if (ec)
+       return false;
       missing.pop();
     }
-  while (!missing.empty() && !ec);
+  while (!missing.empty());
 
-  return missing.empty();
+  return created;
 }
 
 namespace

diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directories.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directories.cc
index 8b9b1b37bb9d959a0654fbfad8b4067c9a48fa63..a85a15eaea836edf9d6528f899e18ced979352c2 100644 (file)
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directories.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/create_directories.cc
@@ -76,8 +76,59 @@ test01()
   VERIFY( count == 6 );
 }
 
+void
+test02()
+{
+  // PR libstdc++/86910
+  const auto p = __gnu_test::nonexistent_path();
+  std::error_code ec;
+  bool result;
+
+  {
+    __gnu_test::scoped_file file;
+
+    result = create_directories(file.path, ec);
+    VERIFY( !result );
+    VERIFY( ec == std::errc::not_a_directory );
+    result = create_directories(file.path / "foo", ec);
+    VERIFY( !result );
+    VERIFY( ec == std::errc::not_a_directory );
+  }
+
+  create_directories(p);
+  {
+    __gnu_test::scoped_file dir(p, __gnu_test::scoped_file::adopt_file);
+    __gnu_test::scoped_file file(dir.path/"file");
+
+    result = create_directories(file.path, ec);
+    VERIFY( !result );
+    VERIFY( ec == std::errc::not_a_directory );
+    result = create_directories(file.path/"../bar", ec);
+    VERIFY( !result );
+    VERIFY( ec == std::errc::not_a_directory );
+  }
+}
+
+void
+test03()
+{
+  // PR libstdc++/87846
+  const auto p = __gnu_test::nonexistent_path() / "";
+  bool result = create_directories(p);
+  VERIFY( result );
+  VERIFY( exists(p) );
+  remove(p);
+  result = create_directories(p/"foo/");
+  VERIFY( result );
+  VERIFY( exists(p) );
+  VERIFY( exists(p/"foo") );
+  remove_all(p);
+}
+
 int
 main()
 {
   test01();
+  test02();
+  test03();
 }

diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directories.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directories.cc
index c81b0b8e11f820a8ff6a603f184ecb00d49d9ee1..e25ccb6c4f1c25c484f484d02526427dbb61e0b0 100644 (file)
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directories.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/create_directories.cc
@@ -69,8 +69,60 @@ test01()
   VERIFY( count == 6 );
 }
 
+void
+test02()
+{
+  // PR libstdc++/86910
+  const auto p = __gnu_test::nonexistent_path();
+  std::error_code ec;
+  bool result;
+
+  {
+    __gnu_test::scoped_file file;
+
+    result = create_directories(file.path, ec);
+    VERIFY( !result );
+    VERIFY( ec == std::errc::not_a_directory );
+    result = create_directories(file.path / "foo", ec);
+    VERIFY( !result );
+    __builtin_printf("%d\n", ec.value());
+    VERIFY( ec == std::errc::not_a_directory );
+  }
+
+  create_directories(p);
+  {
+    __gnu_test::scoped_file dir(p, __gnu_test::scoped_file::adopt_file);
+    __gnu_test::scoped_file file(dir.path/"file");
+
+    result = create_directories(file.path, ec);
+    VERIFY( !result );
+    VERIFY( ec == std::errc::not_a_directory );
+    result = create_directories(file.path/"../bar", ec);
+    VERIFY( !result );
+    VERIFY( ec == std::errc::not_a_directory );
+  }
+}
+
+void
+test03()
+{
+  // PR libstdc++/87846
+  const auto p = __gnu_test::nonexistent_path() / "/";
+  bool result = create_directories(p);
+  VERIFY( result );
+  VERIFY( exists(p) );
+  remove(p);
+  result = create_directories(p/"foo/");
+  VERIFY( result );
+  VERIFY( exists(p) );
+  VERIFY( exists(p/"foo") );
+  remove_all(p);
+}
+
 int
 main()
 {
   test01();
+  test02();
+  test03();
 }