From 1eef21ccfa5988a23a3b71a99613f27ea6a26da6 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Tue, 28 Jun 2022 16:09:21 +0100 Subject: [PATCH] libstdc++: Improve exceptions thrown from fs::temp_directory_path Currently the throwing overload of fs::temp_directory_path() will discard the path that was obtained from the environment. When it fails because the path doesn't resolve to a directory you get an unhelpful error like: filesystem error: temp_directory_path: Not a directory It would be better to also print the path in that case, e.g. filesystem error: temp_directory_path: Not a directory [/home/bob/tmp] libstdc++-v3/ChangeLog: * src/c++17/fs_ops.cc (fs::temp_directory_path()): Include path in exception. (fs::temp_directory_path(error_code&)): Rearrange to more closely match the structure of the first overload. * src/filesystem/ops.cc (fs::temp_directory_path): Likewise. * testsuite/27_io/filesystem/operations/temp_directory_path.cc: Check that exception contains the path. * testsuite/experimental/filesystem/operations/temp_directory_path.cc: Likewise. --- libstdc++-v3/src/c++17/fs_ops.cc | 34 +++++++++++++------ libstdc++-v3/src/filesystem/ops.cc | 31 ++++++++++------- .../operations/temp_directory_path.cc | 5 +++ .../operations/temp_directory_path.cc | 5 +++ 4 files changed, 52 insertions(+), 23 deletions(-) diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc index 435368fa5c5..ed5e9f7d5cf 100644 --- a/libstdc++-v3/src/c++17/fs_ops.cc +++ b/libstdc++-v3/src/c++17/fs_ops.cc @@ -1571,25 +1571,37 @@ fs::path fs::temp_directory_path() { error_code ec; - path tmp = temp_directory_path(ec); + path p = fs::get_temp_directory_from_env(ec); + if (!ec) + { + auto st = status(p, ec); + if (!ec && !is_directory(st)) + ec = std::make_error_code(std::errc::not_a_directory); + } if (ec) - _GLIBCXX_THROW_OR_ABORT(filesystem_error("temp_directory_path", ec)); - return tmp; + { + if (p.empty()) + _GLIBCXX_THROW_OR_ABORT(filesystem_error("temp_directory_path", ec)); + else + _GLIBCXX_THROW_OR_ABORT(filesystem_error("temp_directory_path", p, ec)); + } + return p; } fs::path fs::temp_directory_path(error_code& ec) { path p = fs::get_temp_directory_from_env(ec); - if (ec) - return p; - auto st = status(p, ec); - if (ec) - p.clear(); - else if (!is_directory(st)) + if (!ec) { - p.clear(); - ec = std::make_error_code(std::errc::not_a_directory); + auto st = status(p, ec); + if (ec) + p.clear(); + else if (!is_directory(st)) + { + p.clear(); + ec = std::make_error_code(std::errc::not_a_directory); + } } return p; } diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index 896a4918ace..ab84eb84594 100644 --- a/libstdc++-v3/src/filesystem/ops.cc +++ b/libstdc++-v3/src/filesystem/ops.cc @@ -1326,25 +1326,32 @@ fs::path fs::temp_directory_path() { error_code ec; - path tmp = temp_directory_path(ec); - if (ec.value()) - _GLIBCXX_THROW_OR_ABORT(filesystem_error("temp_directory_path", ec)); - return tmp; + path p = fs::get_temp_directory_from_env(ec); + if (!ec) + { + auto st = status(p, ec); + if (!ec && !is_directory(st)) + ec = std::make_error_code(std::errc::not_a_directory); + } + if (ec) + _GLIBCXX_THROW_OR_ABORT(filesystem_error("temp_directory_path", p, ec)); + return p; } fs::path fs::temp_directory_path(error_code& ec) { path p = fs::get_temp_directory_from_env(ec); - if (ec) - return p; - auto st = status(p, ec); - if (ec) - p.clear(); - else if (!is_directory(st)) + if (!ec) { - p.clear(); - ec = std::make_error_code(std::errc::not_a_directory); + auto st = status(p, ec); + if (ec) + p.clear(); + else if (!is_directory(st)) + { + p.clear(); + ec = std::make_error_code(std::errc::not_a_directory); + } } return p; } diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/temp_directory_path.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/temp_directory_path.cc index b4ef77f05e4..56bd7408c2d 100644 --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/temp_directory_path.cc +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/temp_directory_path.cc @@ -140,12 +140,17 @@ test04() VERIFY( r == fs::path() ); std::error_code ec2; + std::string failed_path; try { fs::temp_directory_path(); } catch (const fs::filesystem_error& e) { ec2 = e.code(); + // On Windows the returned path will be in preferred form, i.e. using L'\\' + // and will have a trailing slash, so compare generic forms. + failed_path = e.path1().generic_string(); } VERIFY( ec2 == ec ); + VERIFY( failed_path.find(f.path.generic_string()) != std::string::npos ); } int diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc index c2945c90866..1772f9737c1 100644 --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc @@ -141,12 +141,17 @@ test04() VERIFY( r == fs::path() ); std::error_code ec2; + std::string failed_path; try { fs::temp_directory_path(); } catch (const fs::filesystem_error& e) { ec2 = e.code(); + // On Windows the returned path will be in preferred form, i.e. using L'\\' + // and will have a trailing slash, so compare generic forms. + failed_path = e.path1().generic_string(); } VERIFY( ec2 == ec ); + VERIFY( failed_path.find(f.path.generic_string()) != std::string::npos ); } int