PR libstdc++/83279 handle sendfile not copying entire file

Message ID 20171214215301.GA26373@redhat.com
State New
Headers show
Series
  • PR libstdc++/83279 handle sendfile not copying entire file
Related show

Commit Message

Jonathan Wakely Dec. 14, 2017, 9:53 p.m.
I failed to notice that the man page for sendfile(3) says it won't
copy more than 2GiB. This refactors the code to first try sendfile
(because it's fast if it works) and if that fails, or stops before the
end of the file, then use filebufs to copy what's left over.

I'm not adding a test because creating two files larger than 2G isn't
a good idea, but I've tested it locally.

	PR libstdc++/83279
	* src/filesystem/std-ops.cc (do_copy_file): Handle sendfile not
	copying entire file.

Tested x86_64-linux, committed to trunk. Backports to follow.
commit ae3ff01506b1bb3232be1063b3fc7df1c80994cb
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Dec 14 21:29:29 2017 +0000

    PR libstdc++/83279 handle sendfile not copying entire file
    
            PR libstdc++/83279
            * src/filesystem/std-ops.cc (do_copy_file): Handle sendfile not
            copying entire file.

Comments

Jonathan Wakely Jan. 5, 2018, 9:44 p.m. | #1
On 14/12/17 21:53 +0000, Jonathan Wakely wrote:
>I failed to notice that the man page for sendfile(3) says it won't

>copy more than 2GiB. This refactors the code to first try sendfile

>(because it's fast if it works) and if that fails, or stops before the

>end of the file, then use filebufs to copy what's left over.

>

>I'm not adding a test because creating two files larger than 2G isn't

>a good idea, but I've tested it locally.

>

>	PR libstdc++/83279

>	* src/filesystem/std-ops.cc (do_copy_file): Handle sendfile not

>	copying entire file.



This restores the non-null offset argument to sendfile, which is
required by Solaris.

Tested x86_64-linux, committed to trunk.
commit d448551ca3cbbfd35796609128d54c8a0030a032
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jan 5 21:04:54 2018 +0000

    PR libstdc++/83279 Use non-null offset argument for sendfile
    
            PR libstdc++/83279
            * src/filesystem/std-ops.cc  (do_copy_file): Use non-null offset with
            sendfile.

diff --git a/libstdc++-v3/src/filesystem/std-ops.cc b/libstdc++-v3/src/filesystem/std-ops.cc
index 2411bbb7977..bed1ad1fe27 100644
--- a/libstdc++-v3/src/filesystem/std-ops.cc
+++ b/libstdc++-v3/src/filesystem/std-ops.cc
@@ -382,10 +382,10 @@ fs::do_copy_file(const char* from, const char* to,
       return false;
     }
 
-  ssize_t n = 0;
   size_t count = from_st->st_size;
 #ifdef _GLIBCXX_USE_SENDFILE
-  n = ::sendfile(out.fd, in.fd, nullptr, count);
+  off_t offset = 0;
+  ssize_t n = ::sendfile(out.fd, in.fd, &offset, count);
   if (n < 0 && errno != ENOSYS && errno != EINVAL)
     {
       ec.assign(errno, std::generic_category());
@@ -405,35 +405,32 @@ fs::do_copy_file(const char* from, const char* to,
     count -= n;
 #endif // _GLIBCXX_USE_SENDFILE
 
-  __gnu_cxx::stdio_filebuf<char> sbin(in.fd, std::ios::in);
-  __gnu_cxx::stdio_filebuf<char> sbout(out.fd, std::ios::out);
+  using std::ios;
+  __gnu_cxx::stdio_filebuf<char> sbin(in.fd, ios::in|ios::binary);
+  __gnu_cxx::stdio_filebuf<char> sbout(out.fd, ios::out|ios::binary);
 
   if (sbin.is_open())
     in.fd = -1;
   if (sbout.is_open())
     out.fd = -1;
 
-  const std::streampos errpos(std::streamoff(-1));
-
-  if (n < 0)
+#ifdef _GLIBCXX_USE_SENDFILE
+  if (n != 0)
     {
-      auto p1 = sbin.pubseekoff(0, std::ios_base::beg, std::ios_base::in);
-      auto p2 = sbout.pubseekoff(0, std::ios_base::beg, std::ios_base::out);
+      if (n < 0)
+	n = 0;
+
+      const auto p1 = sbin.pubseekoff(n, ios::beg, ios::in);
+      const auto p2 = sbout.pubseekoff(n, ios::beg, ios::out);
+
+      const std::streampos errpos(std::streamoff(-1));
       if (p1 == errpos || p2 == errpos)
 	{
 	  ec = std::make_error_code(std::errc::io_error);
 	  return false;
 	}
     }
-  else if (n > 0)
-    {
-      auto p = sbout.pubseekoff(n, std::ios_base::beg, std::ios_base::out);
-      if (p == errpos)
-	{
-	  ec = std::make_error_code(std::errc::io_error);
-	  return false;
-	}
-    }
+#endif
 
   if (count && !(std::ostream(&sbout) << &sbin))
     {

Patch

diff --git a/libstdc++-v3/src/filesystem/std-ops.cc b/libstdc++-v3/src/filesystem/std-ops.cc
index fa5e19a36ba..a15857c31bf 100644
--- a/libstdc++-v3/src/filesystem/std-ops.cc
+++ b/libstdc++-v3/src/filesystem/std-ops.cc
@@ -382,48 +382,71 @@  fs::do_copy_file(const char* from, const char* to,
       return false;
     }
 
+  ssize_t n = 0;
+  size_t count = from_st->st_size;
 #ifdef _GLIBCXX_USE_SENDFILE
-  off_t offset = 0;
-  const auto n = ::sendfile(out.fd, in.fd, &offset, from_st->st_size);
-  if (n < 0 && (errno == ENOSYS || errno == EINVAL))
+  n = ::sendfile(out.fd, in.fd, nullptr, count);
+  if (n < 0 && errno != ENOSYS && errno != EINVAL)
     {
-#endif // _GLIBCXX_USE_SENDFILE
-      __gnu_cxx::stdio_filebuf<char> sbin(in.fd, std::ios::in);
-      __gnu_cxx::stdio_filebuf<char> sbout(out.fd, std::ios::out);
-      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);
-	  return false;
-	}
-      if (!sbout.close() || !sbin.close())
+      ec.assign(errno, std::generic_category());
+      return false;
+    }
+  if ((size_t)n == count)
+    {
+      if (!out.close() || !in.close())
 	{
 	  ec.assign(errno, std::generic_category());
 	  return false;
 	}
-
       ec.clear();
       return true;
-
-#ifdef _GLIBCXX_USE_SENDFILE
     }
-  if (n != from_st->st_size)
+  else if (n > 0)
+    count -= n;
+#endif // _GLIBCXX_USE_SENDFILE
+
+  __gnu_cxx::stdio_filebuf<char> sbin(in.fd, std::ios::in);
+  __gnu_cxx::stdio_filebuf<char> sbout(out.fd, std::ios::out);
+
+  if (sbin.is_open())
+    in.fd = -1;
+  if (sbout.is_open())
+    out.fd = -1;
+
+  const std::streampos errpos(std::streamoff(-1));
+
+  if (n < 0)
+    {
+      auto p1 = sbin.pubseekoff(0, std::ios_base::beg, std::ios_base::in);
+      auto p2 = sbout.pubseekoff(0, std::ios_base::beg, std::ios_base::out);
+      if (p1 == errpos || p2 == errpos)
+	{
+	  ec = std::make_error_code(std::errc::io_error);
+	  return false;
+	}
+    }
+  else if (n > 0)
+    {
+      auto p = sbout.pubseekoff(n, std::ios_base::beg, std::ios_base::out);
+      if (p == errpos)
+	{
+	  ec = std::make_error_code(std::errc::io_error);
+	  return false;
+	}
+    }
+
+  if (count && !(std::ostream(&sbout) << &sbin))
+    {
+      ec = std::make_error_code(std::errc::io_error);
+      return false;
+    }
+  if (!sbout.close() || !sbin.close())
     {
       ec.assign(errno, std::generic_category());
       return false;
     }
-  if (!out.close() || !in.close())
-    {
-      ec.assign(errno, std::generic_category());
-      return false;
-    }
-
   ec.clear();
   return true;
-#endif // _GLIBCXX_USE_SENDFILE
 }
 #endif // NEED_DO_COPY_FILE
 #endif // _GLIBCXX_HAVE_SYS_STAT_H