Commit 14905251 by Jonathan Wakely Committed by Jonathan Wakely

libstdc++/70609 fix filesystem::copy()

	PR libstdc++/70609
	* src/filesystem/ops.cc (close_fd): New function.
	(do_copy_file): Set permissions before copying file contents. Check
	result of closing file descriptors. Don't copy streambuf when file
	is empty.
	(copy(const path&, const path&, copy_options, error_code&)): Use
	lstat for source file when copy_symlinks is set.
	* testsuite/experimental/filesystem/operations/copy.cc: Test copy().

From-SVN: r235215
parent 9c476ad4
2016-04-19 Jonathan Wakely <jwakely@redhat.com> 2016-04-19 Jonathan Wakely <jwakely@redhat.com>
PR libstdc++/70609
* src/filesystem/ops.cc (close_fd): New function.
(do_copy_file): Set permissions before copying file contents. Check
result of closing file descriptors. Don't copy streambuf when file
is empty.
(copy(const path&, const path&, copy_options, error_code&)): Use
lstat for source file when copy_symlinks is set.
* testsuite/experimental/filesystem/operations/copy.cc: Test copy().
* include/experimental/bits/fs_fwd.h (operator&, operator|, operator^, * include/experimental/bits/fs_fwd.h (operator&, operator|, operator^,
operator~ operator&=, operator|=, operator^=): Add noexcept to operator~ operator&=, operator|=, operator^=): Add noexcept to
overloaded operators for copy_options, perms and directory_options. overloaded operators for copy_options, perms and directory_options.
......
...@@ -300,6 +300,17 @@ namespace ...@@ -300,6 +300,17 @@ namespace
}; };
} }
// 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 bool
do_copy_file(const fs::path& from, const fs::path& to, do_copy_file(const fs::path& from, const fs::path& to,
fs::copy_options option, fs::copy_options option,
...@@ -376,7 +387,8 @@ namespace ...@@ -376,7 +387,8 @@ namespace
} }
struct CloseFD { struct CloseFD {
~CloseFD() { if (fd != -1) ::close(fd); } ~CloseFD() { if (fd != -1) close_fd(fd); }
bool close() { return close_fd(std::exchange(fd, -1)); }
int fd; int fd;
}; };
...@@ -401,34 +413,49 @@ namespace ...@@ -401,34 +413,49 @@ namespace
return false; return false;
} }
#ifdef _GLIBCXX_USE_FCHMOD
if (::fchmod(out.fd, from_st->st_mode))
#elif _GLIBCXX_USE_FCHMODAT
if (::fchmodat(AT_FDCWD, to.c_str(), from_st->st_mode, 0))
#else
if (::chmod(to.c_str(), from_st->st_mode))
#endif
{
ec.assign(errno, std::generic_category());
return false;
}
#ifdef _GLIBCXX_USE_SENDFILE #ifdef _GLIBCXX_USE_SENDFILE
auto n = ::sendfile(out.fd, in.fd, nullptr, from_st->st_size); const auto n = ::sendfile(out.fd, in.fd, nullptr, from_st->st_size);
if (n != from_st->st_size) if (n != from_st->st_size)
{ {
ec.assign(errno, std::generic_category()); ec.assign(errno, std::generic_category());
return false; return false;
} }
if (!out.close() || !in.close())
{
ec.assign(errno, std::generic_category());
return false;
}
#else #else
__gnu_cxx::stdio_filebuf<char> sbin(in.fd, std::ios::in); __gnu_cxx::stdio_filebuf<char> sbin(in.fd, std::ios::in);
__gnu_cxx::stdio_filebuf<char> sbout(out.fd, std::ios::out); __gnu_cxx::stdio_filebuf<char> sbout(out.fd, std::ios::out);
if ( !(std::ostream(&sbout) << &sbin) ) if (sbin.is_open())
in.fd = -1;
if (sbout.is_open())
out.fd = -1;
if (from_st->st_size && !(std::ostream(&sbout) << &sbin))
{ {
ec = std::make_error_code(std::errc::io_error); ec = std::make_error_code(std::errc::io_error);
return false; return false;
} }
#endif if (sbout.close() || sbin.close())
#ifdef _GLIBCXX_USE_FCHMOD
if (::fchmod(out.fd, from_st->st_mode))
#elif _GLIBCXX_USE_FCHMODAT
if (::fchmodat(AT_FDCWD, to.c_str(), from_st->st_mode, 0))
#else
if (::chmod(to.c_str(), from_st->st_mode))
#endif
{ {
ec.assign(errno, std::generic_category()); ec.assign(errno, std::generic_category());
return false; return false;
} }
#endif
ec.clear(); ec.clear();
return true; return true;
} }
...@@ -439,13 +466,15 @@ void ...@@ -439,13 +466,15 @@ void
fs::copy(const path& from, const path& to, copy_options options, fs::copy(const path& from, const path& to, copy_options options,
error_code& ec) noexcept error_code& ec) noexcept
{ {
bool skip_symlinks = is_set(options, copy_options::skip_symlinks); const bool skip_symlinks = is_set(options, copy_options::skip_symlinks);
bool create_symlinks = is_set(options, copy_options::create_symlinks); const bool create_symlinks = is_set(options, copy_options::create_symlinks);
bool use_lstat = create_symlinks || skip_symlinks; const bool copy_symlinks = is_set(options, copy_options::copy_symlinks);
const bool use_lstat = create_symlinks || skip_symlinks;
file_status f, t; file_status f, t;
stat_type from_st, to_st; stat_type from_st, to_st;
if (use_lstat // N4099 doesn't check copy_symlinks here, but I think that's a defect.
if (use_lstat || copy_symlinks
? ::lstat(from.c_str(), &from_st) ? ::lstat(from.c_str(), &from_st)
: ::stat(from.c_str(), &from_st)) : ::stat(from.c_str(), &from_st))
{ {
...@@ -488,7 +517,7 @@ fs::copy(const path& from, const path& to, copy_options options, ...@@ -488,7 +517,7 @@ fs::copy(const path& from, const path& to, copy_options options,
{ {
if (skip_symlinks) if (skip_symlinks)
ec.clear(); ec.clear();
else if (!exists(t) && is_set(options, copy_options::copy_symlinks)) else if (!exists(t) && copy_symlinks)
copy_symlink(from, to, ec); copy_symlink(from, to, ec);
else else
// Not clear what should be done here. // Not clear what should be done here.
......
...@@ -21,34 +21,127 @@ ...@@ -21,34 +21,127 @@
// 15.3 Copy [fs.op.copy] // 15.3 Copy [fs.op.copy]
#include <experimental/filesystem> #include <experimental/filesystem>
#include <fstream>
#include <testsuite_fs.h> #include <testsuite_fs.h>
#include <testsuite_hooks.h> #include <testsuite_hooks.h>
using std::experimental::filesystem::path; namespace fs = std::experimental::filesystem;
// Test error conditions.
void void
test01() test01()
{ {
bool test __attribute__((unused)) = false; bool test __attribute__((unused)) = false;
for (const path& p : __gnu_test::test_paths) auto p = __gnu_test::nonexistent_path();
VERIFY( absolute(p).is_absolute() ); std::error_code ec;
VERIFY( !fs::exists(p) );
fs::copy(p, ".", fs::copy_options::none, ec);
VERIFY( ec );
ec.clear();
fs::copy(".", ".", fs::copy_options::none, ec);
VERIFY( ec );
std::ofstream{p.native()};
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);
} }
// Test is_symlink(f) case.
void void
test02() test02()
{ {
bool test __attribute__((unused)) = false; bool test __attribute__((unused)) = false;
path p1("/"); auto from = __gnu_test::nonexistent_path();
VERIFY( absolute(p1) == p1 ); auto to = __gnu_test::nonexistent_path();
VERIFY( absolute(p1, "/bar") == p1 ); std::error_code ec;
path p2("/foo");
VERIFY( absolute(p2) == p2 ); fs::create_symlink(".", from, ec);
VERIFY( absolute(p2, "/bar") == p2 ); VERIFY( !ec );
path p3("foo"); VERIFY( fs::exists(from) );
VERIFY( absolute(p3) != p3 );
VERIFY( absolute(p3, "/bar") == "/bar/foo" ); fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
VERIFY( !ec );
VERIFY( !fs::exists(to) );
fs::copy(from, to, fs::copy_options::skip_symlinks, ec);
VERIFY( !ec );
VERIFY( !fs::exists(to) );
fs::copy(from, to,
fs::copy_options::skip_symlinks|fs::copy_options::copy_symlinks,
ec);
VERIFY( !ec );
VERIFY( !fs::exists(to) );
fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
VERIFY( !ec );
VERIFY( fs::exists(to) );
fs::copy(from, to, fs::copy_options::copy_symlinks, ec);
VERIFY( ec );
remove(from, ec);
remove(to, ec);
}
// Test is_regular_file(f) case.
void
test03()
{
bool test __attribute__((unused)) = false;
auto from = __gnu_test::nonexistent_path();
auto to = __gnu_test::nonexistent_path();
// test empty file
std::ofstream{from.native()};
VERIFY( fs::exists(from) );
VERIFY( fs::file_size(from) == 0 );
fs::copy(from, to);
VERIFY( fs::exists(to) );
VERIFY( fs::file_size(to) == 0 );
remove(to);
VERIFY( !fs::exists(to) );
std::ofstream{from.native()} << "Hello, filesystem!";
VERIFY( fs::file_size(from) != 0 );
fs::copy(from, to);
VERIFY( fs::exists(to) );
VERIFY( fs::file_size(to) == fs::file_size(from) );
}
// Test is_directory(f) case.
void
test04()
{
bool test __attribute__((unused)) = false;
auto from = __gnu_test::nonexistent_path();
auto to = __gnu_test::nonexistent_path();
std::error_code ec;
}
// Test no-op cases.
void
test05()
{
bool test __attribute__((unused)) = false;
auto to = __gnu_test::nonexistent_path();
std::error_code ec;
fs::copy("/", to, fs::copy_options::create_symlinks, ec);
VERIFY( !ec );
} }
int int
...@@ -56,4 +149,7 @@ main() ...@@ -56,4 +149,7 @@ main()
{ {
test01(); test01();
test02(); test02();
test03();
test04();
test05();
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment