From ffe2c05539d5ec6bfc4154fcdc71c41adf900a29 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Thu, 29 Nov 2018 00:39:37 +0000 Subject: [PATCH] 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 --- libstdc++-v3/ChangeLog | 14 ++++ libstdc++-v3/src/filesystem/ops.cc | 32 ++++++-- libstdc++-v3/src/filesystem/std-ops.cc | 80 ++++++++++++++----- .../operations/create_directories.cc | 51 ++++++++++++ .../operations/create_directories.cc | 52 ++++++++++++ 5 files changed, 200 insertions(+), 29 deletions(-) diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index 1107da0f53e..607edb9beb3 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,3 +1,17 @@ +2018-11-29 Jonathan Wakely + + 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 PR libstdc++/83306 diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index 40cadbf6270..a6ae75ff734 100644 --- 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 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 e266fa6d3f8..2f9a76ffaa1 100644 --- 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 missing; path pp = p; - while (pp.has_filename() && status(pp, ec).type() == file_type::not_found) - { - ec.clear(); - const auto& filename = pp.filename(); - if (!is_dot(filename) && !is_dotdot(filename)) - missing.push(pp); - pp = pp.parent_path(); - - if (missing.size() > 1000) // sanity check - { - ec = std::make_error_code(std::errc::filename_too_long); - return false; - } - } - - if (ec || missing.empty()) - return false; + // Strip any trailing slash + if (pp.has_relative_path() && !pp.has_filename()) + pp = pp.parent_path(); + do + { + const auto& filename = pp.filename(); + 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 (pp.empty()) + break; + + st = status(pp, ec); + if (exists(st)) + { + if (ec) + return false; + if (!is_directory(st)) + { + ec = std::make_error_code(std::errc::not_a_directory); + 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 8b9b1b37bb9..a85a15eaea8 100644 --- 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 c81b0b8e11f..e25ccb6c4f1 100644 --- 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(); }