]> git.eshelyaron.com Git - emacs.git/commitdiff
copy-file no longer trusts st_size
authorPaul Eggert <eggert@cs.ucla.edu>
Wed, 22 Jan 2025 19:06:06 +0000 (11:06 -0800)
committerEshel Yaron <me@eshelyaron.com>
Thu, 24 Jul 2025 08:47:21 +0000 (10:47 +0200)
In copy-file, do not trust st_size, since it might change as we run,
or we might be in a /proc system where it is unreliable anyway.
Also, fix some other unlikely copy-file bugs while we’re here.
* src/fileio.c (Fcopy_file): Use O_TRUNC when opening a
destination that already exists.  This saves us the trouble
of having to call ftruncate with a possibly-bogus st_size;
the old motivation for using ftruncate is no longer compelling.
Do not assume ptrdiff_t is as wide as ssize_t; although this is
true on all known platforms, it’s easy to not assume it.
Don’t trust st_size.  Prefer SSIZE_MAX to TYPE_MAXIMUM (ssize_t).
Always read+write, regardless of whether copy_file_range failed.

(cherry picked from commit ffd65be2277b9a30e77a00ad69c9ba21459f72c5)

src/fileio.c

index 4c3bc5787cfe9a8ba3490853cf6f0cf716e690c4..82ee22d21876f72ed2e208d924f002e8e2f81f34 100644 (file)
@@ -2369,15 +2369,13 @@ permissions.  */)
        barf_or_query_if_file_exists (newname, true, "copy to it",
                                      FIXNUMP (ok_if_already_exists), false);
       already_exists = true;
-      ofd = emacs_open (SSDATA (encoded_newname), O_WRONLY, 0);
+      ofd = emacs_open (SSDATA (encoded_newname), O_WRONLY | O_TRUNC, 0);
     }
   if (ofd < 0)
     report_file_error ("Opening output file", newname);
 
   record_unwind_protect_int (close_file_unwind, ofd);
 
-  off_t oldsize = 0, newsize;
-
   if (already_exists)
     {
       struct stat out_st;
@@ -2386,36 +2384,26 @@ permissions.  */)
       if (st.st_dev == out_st.st_dev && st.st_ino == out_st.st_ino)
        report_file_errno ("Input and output files are the same",
                           list2 (file, newname), 0);
-      if (S_ISREG (out_st.st_mode))
-       oldsize = out_st.st_size;
     }
 
   maybe_quit ();
 
-  if (emacs_fd_to_int (ifd) != -1
-      && clone_file (ofd, emacs_fd_to_int (ifd)))
-    newsize = st.st_size;
-  else
+  if (emacs_fd_to_int (ifd) == -1
+      || !clone_file (ofd, emacs_fd_to_int (ifd)))
     {
-      off_t insize = st.st_size;
-      ssize_t copied;
+      off_t newsize = 0;
 
 #ifndef MSDOS
-      newsize = 0;
-
       if (emacs_fd_to_int (ifd) != -1)
        {
-         for (; newsize < insize; newsize += copied)
+         for (ssize_t copied; ; newsize += copied)
            {
              /* Copy at most COPY_MAX bytes at a time; this is min
-                (PTRDIFF_MAX, SIZE_MAX) truncated to a value that is
+                (SSIZE_MAX, SIZE_MAX) truncated to a value that is
                 surely aligned well.  */
-             ssize_t ssize_max = TYPE_MAXIMUM (ssize_t);
-             ptrdiff_t copy_max = min (ssize_max, SIZE_MAX) >> 30 << 30;
-             off_t intail = insize - newsize;
-             ptrdiff_t len = min (intail, copy_max);
+             ssize_t copy_max = min (SSIZE_MAX, SIZE_MAX) >> 30 << 30;
              copied = copy_file_range (emacs_fd_to_int (ifd), NULL,
-                                       ofd, NULL, len, 0);
+                                       ofd, NULL, copy_max, 0);
              if (copied <= 0)
                break;
              maybe_quit ();
@@ -2423,32 +2411,24 @@ permissions.  */)
        }
 #endif /* MSDOS */
 
-      /* Fall back on read+write if copy_file_range failed, or if the
-        input is empty and so could be a /proc file, or if ifd is an
-        invention of android.c.  read+write will either succeed, or
-        report an error more precisely than copy_file_range
-        would.  */
-      if (newsize != insize || insize == 0)
-       {
-         char buf[MAX_ALLOCA];
+      /* Follow up with read+write regardless of any copy_file_range failure.
+        Many copy_file_range implementations fail for no good reason,
+        or "succeed" even when they did nothing (e.g., in /proc files).
+        Also, if read+write fails it will report an error more
+        precisely than copy_file_range would.  */
+      char buf[MAX_ALLOCA];
 
-         for (; (copied = emacs_fd_read (ifd, buf, sizeof buf));
-              newsize += copied)
-           {
-             if (copied < 0)
-               report_file_error ("Read error", file);
-             if (emacs_write_quit (ofd, buf, copied) != copied)
-               report_file_error ("Write error", newname);
-           }
+      for (ptrdiff_t copied;
+          (copied = emacs_fd_read (ifd, buf, sizeof buf));
+          newsize += copied)
+       {
+         if (copied < 0)
+           report_file_error ("Read error", file);
+         if (emacs_write_quit (ofd, buf, copied) != copied)
+           report_file_error ("Write error", newname);
        }
     }
 
-  /* Truncate any existing output file after writing the data.  This
-     is more likely to work than truncation before writing, if the
-     file system is out of space or the user is over disk quota.  */
-  if (newsize < oldsize && ftruncate (ofd, newsize) != 0)
-    report_file_error ("Truncating output file", newname);
-
 #ifndef MSDOS
   /* Preserve the original file permissions, and if requested, also its
      owner and group.  */