public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jonathan Wakely <jwakely@redhat.com>
To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: [committed] libstdc++: Improve exceptions thrown from fs::temp_directory_path
Date: Thu, 30 Jun 2022 14:18:41 +0100	[thread overview]
Message-ID: <20220630131841.1237664-1-jwakely@redhat.com> (raw)

Tested x86_64-linux and x86_64-mingw, pushed to trunk.

-- >8 --

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
-- 
2.36.1


                 reply	other threads:[~2022-06-30 13:19 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220630131841.1237664-1-jwakely@redhat.com \
    --to=jwakely@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).