This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
[PATCH] Five patches for std::experimental::filesystem
- From: Jonathan Wakely <jwakely at redhat dot com>
- To: libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
- Date: Mon, 24 Oct 2016 17:50:10 +0100
- Subject: [PATCH] Five patches for std::experimental::filesystem
- Authentication-results: sourceware.org; auth=none
These implement DR resolutions and fix bugs.
Fix error handling in filesystem::is_empty
* src/filesystem/ops.cc (is_empty): Fix error handling.
* testsuite/experimental/filesystem/operations/is_empty.cc: New test.
PR71337 fix filesystem::temp_directory_path error handling
PR libstdc++/71337
* src/filesystem/ops.cc (temp_directory_path): Pass error_code
argument to other filesystem operations.
* testsuite/experimental/filesystem/operations/temp_directory_path.cc:
Add testcase for inaccessible directory.
Make directory iterators become end iterator on error
* src/filesystem/dir.cc (open_dir): Return same value for errors
whether ignored or not.
(_Dir::advance(error_code*, directory_options)): Return false on
error.
(directory_iterator(const path&, directory_options, error_code*)):
Create end iterator on error (LWG 2723).
(recursive_directory_iterator(const path&, directory_options,
error_code*)): Likewise.
* testsuite/experimental/filesystem/iterators/directory_iterator.cc:
Update expected behaviour on error.
* testsuite/experimental/filesystem/iterators/
recursive_directory_iterator.cc: Likewise.
Do not retry failed close(3) in filesystem::copy
* src/filesystem/ops.cc (close_fd): Remove.
(do_copy_file): Just use close(3) instead of close_fd, to prevent
retrying on error.
Implement DR resolutions for filesystem::copy
* src/filesystem/ops.cc (do_copy_file): Return an error if either
source or destination is not a regular file.
(copy): Update comment to refer to LWG 2681. Implement 2682 and 2683
resolutions.
(read_symlink): Add missing ec.clear().
* testsuite/experimental/filesystem/operations/copy.cc: Update
expected behaviour for copying directories with create_symlinks.
Verify that error_code arguments are cleared if there's no error.
* testsuite/experimental/filesystem/operations/read_symlink.cc:
* New.
Tested x86_64-linux, commiitted to trunk. I'm going to do some mass
backports for all filesystem fixes to the branches this week as well.
commit 363cdc78be5edb191411e57206d94e18e42ec259
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Oct 24 15:50:51 2016 +0100
Fix error handling in filesystem::is_empty
* src/filesystem/ops.cc (is_empty): Fix error handling.
* testsuite/experimental/filesystem/operations/is_empty.cc: New test.
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 90c225b..d9a12df 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1022,20 +1022,24 @@ fs::hard_link_count(const path& p, error_code& ec) noexcept
bool
fs::is_empty(const path& p)
{
- return fs::is_directory(status(p))
- ? fs::directory_iterator(p) == fs::directory_iterator()
- : fs::file_size(p) == 0;
+ error_code ec;
+ bool e = is_empty(p, ec);
+ if (ec)
+ _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot check is file is empty",
+ p, ec));
+ return e;
}
bool
fs::is_empty(const path& p, error_code& ec) noexcept
{
auto s = status(p, ec);
- if (ec.value())
+ if (ec)
return false;
- return fs::is_directory(s)
+ bool empty = fs::is_directory(s)
? fs::directory_iterator(p, ec) == fs::directory_iterator()
: fs::file_size(p, ec) == 0;
+ return ec ? false : empty;
}
fs::file_time_type
commit 4d32e1be1f623b42743092aefb7388019bea0c7f
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Oct 24 15:27:00 2016 +0100
PR71337 fix filesystem::temp_directory_path error handling
PR libstdc++/71337
* src/filesystem/ops.cc (temp_directory_path): Pass error_code
argument to other filesystem operations.
* testsuite/experimental/filesystem/operations/temp_directory_path.cc:
Add testcase for inaccessible directory.
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index f8ba74e..90c225b 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1428,12 +1428,17 @@ fs::path fs::temp_directory_path(error_code& ec)
for (auto e = env; tmpdir == nullptr && *e != nullptr; ++e)
tmpdir = ::getenv(*e);
path p = tmpdir ? tmpdir : "/tmp";
- if (exists(p) && is_directory(p))
+ auto st = status(p, ec);
+ if (!ec)
{
- ec.clear();
- return p;
+ if (is_directory(st))
+ {
+ ec.clear();
+ return p;
+ }
+ else
+ ec = std::make_error_code(std::errc::not_a_directory);
}
- ec = std::make_error_code(std::errc::not_a_directory);
return {};
#endif
}
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 6e202c9..7f7e9fd 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/temp_directory_path.cc
@@ -45,6 +45,7 @@ test01()
std::error_code ec;
fs::path p1 = fs::temp_directory_path(ec);
+ VERIFY( !ec );
VERIFY( exists(p1) );
fs::path p2 = fs::temp_directory_path();
@@ -62,6 +63,7 @@ test02()
std::error_code ec;
fs::path p = fs::temp_directory_path(ec);
VERIFY( ec );
+ VERIFY( p == fs::path() );
std::error_code ec2;
try {
@@ -72,10 +74,54 @@ test02()
VERIFY( ec2 == ec );
}
+void
+test03()
+{
+ auto p = __gnu_test::nonexistent_path();
+ create_directories(p/"tmp");
+ permissions(p, fs::perms::none);
+ setenv("TMPDIR", (p/"tmp").c_str(), 1);
+ std::error_code ec;
+ auto r = fs::temp_directory_path(ec); // libstdc++/PR71337
+ VERIFY( ec == std::make_error_code(std::errc::permission_denied) );
+ VERIFY( r == fs::path() );
+
+ std::error_code ec2;
+ try {
+ fs::temp_directory_path();
+ } catch (const fs::filesystem_error& e) {
+ ec2 = e.code();
+ }
+ VERIFY( ec2 == ec );
+
+ permissions(p, fs::perms::owner_all, ec);
+ remove_all(p, ec);
+}
+
+void
+test04()
+{
+ __gnu_test::scoped_file f;
+ setenv("TMPDIR", f.path.c_str(), 1);
+ std::error_code ec;
+ auto r = fs::temp_directory_path(ec);
+ VERIFY( ec == std::make_error_code(std::errc::not_a_directory) );
+ VERIFY( r == fs::path() );
+
+ std::error_code ec2;
+ try {
+ fs::temp_directory_path();
+ } catch (const fs::filesystem_error& e) {
+ ec2 = e.code();
+ }
+ VERIFY( ec2 == ec );
+}
int
main()
{
test01();
test02();
+ test03();
+ test04();
}
commit 015912fca75b8747123efbf5419e3e8be9b0a5b9
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Oct 24 14:50:01 2016 +0100
Make directory iterators become end iterator on error
* src/filesystem/dir.cc (open_dir): Return same value for errors
whether ignored or not.
(_Dir::advance(error_code*, directory_options)): Return false on
error.
(directory_iterator(const path&, directory_options, error_code*)):
Create end iterator on error (LWG 2723).
(recursive_directory_iterator(const path&, directory_options,
error_code*)): Likewise.
* testsuite/experimental/filesystem/iterators/directory_iterator.cc:
Update expected behaviour on error.
* testsuite/experimental/filesystem/iterators/
recursive_directory_iterator.cc: Likewise.
diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc
index 6ff12d0..4640d75 100644
--- a/libstdc++-v3/src/filesystem/dir.cc
+++ b/libstdc++-v3/src/filesystem/dir.cc
@@ -79,8 +79,7 @@ namespace
return (obj & bits) != Bitmask::none;
}
- // Returns {dirp, p} on success, {nullptr, p} on error.
- // If an ignored EACCES error occurs returns {}.
+ // Returns {dirp, p} on success, {} on error (whether ignored or not).
inline fs::_Dir
open_dir(const fs::path& p, fs::directory_options options,
std::error_code* ec)
@@ -102,7 +101,7 @@ namespace
std::error_code(err, std::generic_category())));
ec->assign(err, std::generic_category());
- return {nullptr, p};
+ return {};
}
inline fs::file_type
@@ -169,7 +168,7 @@ fs::_Dir::advance(error_code* ec, directory_options options)
"directory iterator cannot advance",
std::error_code(err, std::generic_category())));
ec->assign(err, std::generic_category());
- return true;
+ return false;
}
else
{
@@ -191,12 +190,6 @@ directory_iterator(const path& p, directory_options options, error_code* ec)
if (sp->advance(ec, options))
_M_dir.swap(sp);
}
- else if (!dir.path.empty())
- {
- // An error occurred, we need a non-empty shared_ptr so that *this will
- // not compare equal to the end iterator.
- _M_dir.reset(static_cast<fs::_Dir*>(nullptr));
- }
}
const fs::directory_entry&
@@ -270,10 +263,6 @@ recursive_directory_iterator(const path& p, directory_options options,
std::error_code(err, std::generic_category())));
ec->assign(err, std::generic_category());
-
- // An error occurred, we need a non-empty shared_ptr so that *this will
- // not compare equal to the end iterator.
- _M_dirs.reset(static_cast<_Dir_stack*>(nullptr));
}
}
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
index 5c80fb7..5788700 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
@@ -34,7 +34,7 @@ test01()
const auto p = __gnu_test::nonexistent_path();
fs::directory_iterator iter(p, ec);
VERIFY( ec );
- VERIFY( iter != fs::directory_iterator() );
+ VERIFY( iter == fs::directory_iterator() );
// Test empty directory.
create_directory(p, fs::current_path(), ec);
@@ -58,7 +58,7 @@ test01()
VERIFY( !ec );
iter = fs::directory_iterator(p, ec);
VERIFY( ec );
- VERIFY( iter != fs::directory_iterator() );
+ VERIFY( iter == fs::directory_iterator() );
// Test inaccessible directory, skipping permission denied.
const auto opts = fs::directory_options::skip_permission_denied;
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
index 37b2606..b41c394 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
@@ -34,7 +34,7 @@ test01()
const auto p = __gnu_test::nonexistent_path();
fs::recursive_directory_iterator iter(p, ec);
VERIFY( ec );
- VERIFY( iter != fs::recursive_directory_iterator() );
+ VERIFY( iter == fs::recursive_directory_iterator() );
// Test empty directory.
create_directory(p, fs::current_path(), ec);
@@ -60,7 +60,7 @@ test01()
VERIFY( !ec );
iter = fs::recursive_directory_iterator(p, ec);
VERIFY( ec );
- VERIFY( iter != fs::recursive_directory_iterator() );
+ VERIFY( iter == fs::recursive_directory_iterator() );
// Test inaccessible directory, skipping permission denied.
const auto opts = fs::directory_options::skip_permission_denied;
commit ea465ff7461e814a8e2f83007653428a403eadb0
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Oct 24 14:39:24 2016 +0100
Do not retry failed close(3) in filesystem::copy
* src/filesystem/ops.cc (close_fd): Remove.
(do_copy_file): Just use close(3) instead of close_fd, to prevent
retrying on error.
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 6f76053..f8ba74e 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -308,17 +308,6 @@ namespace
return fs::file_time_type{seconds{s} + ns};
}
- // Returns true if the file descriptor was successfully closed,
- // otherwise returns false and the reason will be in errno.
- inline bool
- close_fd(int fd)
- {
- while (::close(fd))
- if (errno != EINTR)
- return false;
- return true;
- }
-
bool
do_copy_file(const fs::path& from, const fs::path& to,
fs::copy_options option,
@@ -405,8 +394,8 @@ namespace
}
struct CloseFD {
- ~CloseFD() { if (fd != -1) close_fd(fd); }
- bool close() { return close_fd(std::exchange(fd, -1)); }
+ ~CloseFD() { if (fd != -1) ::close(fd); }
+ bool close() { return ::close(std::exchange(fd, -1)) == 0; }
int fd;
};
commit 68eea18d5d6aa635f8d1b53e780e7fe703cd7610
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Mon Oct 24 14:33:27 2016 +0100
Implement DR resolutions for filesystem::copy
* src/filesystem/ops.cc (do_copy_file): Return an error if either
source or destination is not a regular file.
(copy): Update comment to refer to LWG 2681. Implement 2682 and 2683
resolutions.
(read_symlink): Add missing ec.clear().
* testsuite/experimental/filesystem/operations/copy.cc: Update
expected behaviour for copying directories with create_symlinks.
Verify that error_code arguments are cleared if there's no error.
* testsuite/experimental/filesystem/operations/read_symlink.cc: New.
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 2286e22..6f76053 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -361,6 +361,11 @@ namespace
from_st = &st2;
}
f = make_file_status(*from_st);
+ if (!is_regular_file(f))
+ {
+ ec = std::make_error_code(std::errc::not_supported);
+ return false;
+ }
using opts = fs::copy_options;
@@ -392,6 +397,11 @@ namespace
ec = std::make_error_code(std::errc::file_exists);
return false;
}
+ else if (!is_regular_file(t))
+ {
+ ec = std::make_error_code(std::errc::not_supported);
+ return false;
+ }
}
struct CloseFD {
@@ -489,7 +499,8 @@ fs::copy(const path& from, const path& to, copy_options options,
file_status f, t;
stat_type from_st, to_st;
- // N4099 doesn't check copy_symlinks here, but I think that's a defect.
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 2681. filesystem::copy() cannot copy symlinks
if (use_lstat || copy_symlinks
? ::lstat(from.c_str(), &from_st)
: ::stat(from.c_str(), &from_st))
@@ -556,6 +567,10 @@ fs::copy(const path& from, const path& to, copy_options options,
do_copy_file(from, to, options, &from_st, ptr, ec);
}
}
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 2682. filesystem::copy() won't create a symlink to a directory
+ else if (is_directory(f) && create_symlinks)
+ ec = std::make_error_code(errc::is_a_directory);
else if (is_directory(f) && (is_set(options, copy_options::recursive)
|| options == copy_options::none))
{
@@ -568,7 +583,10 @@ fs::copy(const path& from, const path& to, copy_options options,
for (const directory_entry& x : directory_iterator(from))
copy(x.path(), to/x.path().filename(), options, ec);
}
- // "Otherwise no effects." (should ec.clear() be called?)
+ // _GLIBCXX_RESOLVE_LIB_DEFECTS
+ // 2683. filesystem::copy() says "no effects"
+ else
+ ec.clear();
}
bool
@@ -1168,6 +1186,7 @@ fs::path fs::read_symlink(const path& p, error_code& ec)
ec.assign(errno, std::generic_category());
return {};
}
+ ec.clear();
return path{buf.data(), buf.data()+len};
#else
ec = std::make_error_code(std::errc::not_supported);
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
index 0d112a2..2cfb1c1 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/copy.cc
@@ -22,7 +22,6 @@
// 15.3 Copy [fs.op.copy]
#include <experimental/filesystem>
-#include <fstream>
#include <testsuite_fs.h>
#include <testsuite_hooks.h>
@@ -43,14 +42,25 @@ test01()
fs::copy(".", ".", fs::copy_options::none, ec);
VERIFY( ec );
- std::ofstream{p.native()};
+ __gnu_test::scoped_file f(p);
VERIFY( fs::is_directory(".") );
VERIFY( fs::is_regular_file(p) );
ec.clear();
fs::copy(".", p, fs::copy_options::none, ec);
VERIFY( ec );
- remove(p, ec);
+ auto to = __gnu_test::nonexistent_path();
+ ec.clear();
+ auto opts = fs::copy_options::create_symlinks;
+ fs::copy("/", to, opts, ec);
+ VERIFY( ec == std::make_error_code(std::errc::is_a_directory) );
+ VERIFY( !exists(to) );
+
+ ec.clear();
+ opts != fs::copy_options::recursive;
+ fs::copy("/", to, opts, ec);
+ VERIFY( ec == std::make_error_code(std::errc::is_a_directory) );
+ VERIFY( !exists(to) );
}
// Test is_symlink(f) case.
@@ -59,29 +69,35 @@ test02()
{
auto from = __gnu_test::nonexistent_path();
auto to = __gnu_test::nonexistent_path();
- std::error_code ec;
+ std::error_code ec, bad = std::make_error_code(std::errc::invalid_argument);
+ ec = bad;
fs::create_symlink(".", from, ec);
VERIFY( !ec );
VERIFY( fs::exists(from) );
+ ec = bad;
fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
VERIFY( !ec );
VERIFY( !fs::exists(to) );
+ ec = bad;
fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
VERIFY( !ec );
VERIFY( !fs::exists(to) );
+ ec = bad;
fs::copy(from, to,
fs::copy_options::skip_symlinks|fs::copy_options::copy_symlinks,
ec);
VERIFY( !ec );
VERIFY( !fs::exists(to) );
+ ec = bad;
fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
VERIFY( !ec );
VERIFY( fs::exists(to) );
+ VERIFY( is_symlink(to) );
fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
VERIFY( ec );
@@ -129,10 +145,10 @@ void
test05()
{
auto to = __gnu_test::nonexistent_path();
- std::error_code ec;
+ std::error_code ec = std::make_error_code(std::errc::invalid_argument);
- fs::copy("/", to, fs::copy_options::create_symlinks, ec);
- VERIFY( !ec );
+ fs::copy("/", to, fs::copy_options::copy_symlinks, ec);
+ VERIFY( !ec ); // Previous value should be cleared (LWG 2683)
}
int
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc
new file mode 100644
index 0000000..c4137bd
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/read_symlink.cc
@@ -0,0 +1,49 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test01()
+{
+ auto p = __gnu_test::nonexistent_path();
+ std::error_code ec;
+
+ read_symlink(p, ec);
+ VERIFY( ec );
+
+ fs::path tgt = ".";
+ create_symlink(tgt, p);
+
+ auto result = read_symlink(p, ec);
+ VERIFY( !ec );
+ VERIFY( result == tgt );
+}
+
+int
+main()
+{
+ test01();
+}