Merge #9709 'fileio: use os_copy to create backups'

ref #8288

6 files changed, 67 insertions, 110 deletions

diff --git a/src/nvim/fileio.c b/src/nvim/fileio.c
index 8e4a210b66..55463bdf30 100644
--- a/src/nvim/fileio.c
+++ b/src/nvim/fileio.c
@@ -2781,20 +2781,12 @@ buf_write (
     else
       backup_ext = p_bex;
 
-    if (backup_copy && (fd = os_open((char *)fname, O_RDONLY, 0)) >= 0) {
-      int bfd;
-      char_u      *copybuf, *wp;
-      int some_error = FALSE;
+    if (backup_copy) {
+      char_u *wp;
+      int some_error = false;
       char_u      *dirp;
       char_u      *rootname;
 
-      copybuf = verbose_try_malloc(BUFSIZE + 1);
-      if (copybuf == NULL) {
-        // out of memory
-        some_error = TRUE;
-        goto nobackup;
-      }
-
       /*
        * Try to make the backup in each directory in the 'bdir' option.
        *
@@ -2812,8 +2804,8 @@ buf_write (
         /*
          * Isolate one directory name, using an entry in 'bdir'.
          */
-        (void)copy_option_part(&dirp, copybuf, BUFSIZE, ",");
-        rootname = get_file_in_dir(fname, copybuf);
+        (void)copy_option_part(&dirp, IObuff, IOSIZE, ",");
+        rootname = get_file_in_dir(fname, IObuff);
         if (rootname == NULL) {
           some_error = TRUE;                /* out of memory */
           goto nobackup;
@@ -2875,87 +2867,30 @@ buf_write (
         if (backup != NULL) {
           /* remove old backup, if present */
           os_remove((char *)backup);
-          /* Open with O_EXCL to avoid the file being created while
-           * we were sleeping (symlink hacker attack?) */
-          bfd = os_open((char *)backup,
-              O_WRONLY|O_CREAT|O_EXCL|O_NOFOLLOW,
-              perm & 0777);
-          if (bfd < 0) {
-            xfree(backup);
-            backup = NULL;
-          } else {
-            // set file protection same as original file, but
-            // strip s-bit.
-            (void)os_setperm((const char *)backup, perm & 0777);
-
-#ifdef UNIX
-            /*
-             * Try to set the group of the backup same as the
-             * original file. If this fails, set the protection
-             * bits for the group same as the protection bits for
-             * others.
-             */
-            if (file_info_new.stat.st_gid != file_info_old.stat.st_gid
-                && os_fchown(bfd, -1, file_info_old.stat.st_gid) != 0) {
-              os_setperm((const char *)backup,
-                         (perm & 0707) | ((perm & 07) << 3));
-            }
-# ifdef HAVE_SELINUX
-            mch_copy_sec(fname, backup);
-# endif
-#endif
 
-            /*
-             * copy the file.
-             */
-            write_info.bw_fd = bfd;
-            write_info.bw_buf = copybuf;
-#ifdef HAS_BW_FLAGS
-            write_info.bw_flags = FIO_NOCONVERT;
-#endif
-            while ((write_info.bw_len = read_eintr(fd, copybuf,
-                        BUFSIZE)) > 0) {
-              if (buf_write_bytes(&write_info) == FAIL) {
-                SET_ERRMSG(_(
-                    "E506: Can't write to backup file (add ! to override)"));
-                break;
-              }
-              os_breakcheck();
-              if (got_int) {
-                SET_ERRMSG(_(e_interr));
-                break;
-              }
-            }
+          // copy the file
+          if (os_copy((char *)fname, (char *)backup, UV_FS_COPYFILE_FICLONE)
+              != 0) {
+            SET_ERRMSG(_("E506: Can't write to backup file "
+                         "(add ! to override)"));
+          }
 
-            int error;
-            if ((error = os_close(bfd)) != 0 && errmsg == NULL) {
-              SET_ERRMSG_ARG(_("E507: Close error for backup file "
-                               "(add ! to override): %s"),
-                             error);
-            }
-            if (write_info.bw_len < 0) {
-              SET_ERRMSG(_(
-                  "E508: Can't read file for backup (add ! to override)"));
-            }
 #ifdef UNIX
-            set_file_time(backup,
-                          file_info_old.stat.st_atim.tv_sec,
-                          file_info_old.stat.st_mtim.tv_sec);
+          set_file_time(backup,
+                        file_info_old.stat.st_atim.tv_sec,
+                        file_info_old.stat.st_mtim.tv_sec);
 #endif
 #ifdef HAVE_ACL
-            mch_set_acl(backup, acl);
+          mch_set_acl(backup, acl);
 #endif
 #ifdef HAVE_SELINUX
-            mch_copy_sec(fname, backup);
+          mch_copy_sec(fname, backup);
 #endif
-            break;
-          }
+          break;
         }
       }
-nobackup:
-      os_close(fd);  // Ignore errors for closing read file.
-      xfree(copybuf);
 
+nobackup:
       if (backup == NULL && errmsg == NULL) {
         SET_ERRMSG(_(
             "E509: Cannot create backup file (add ! to override)"));
@@ -3537,28 +3472,11 @@ restore_backup:
           MSG(_(e_interr));
           ui_flush();
         }
-        if ((fd = os_open((char *)backup, O_RDONLY, 0)) >= 0) {
-          if ((write_info.bw_fd = os_open((char *)fname,
-                                          O_WRONLY | O_CREAT | O_TRUNC,
-                                          perm & 0777)) >= 0) {
-            // copy the file.
-            write_info.bw_buf = smallbuf;
-#ifdef HAS_BW_FLAGS
-            write_info.bw_flags = FIO_NOCONVERT;
-#endif
-            while ((write_info.bw_len = read_eintr(fd, smallbuf,
-                                                   SMBUFSIZE)) > 0) {
-              if (buf_write_bytes(&write_info) == FAIL) {
-                break;
-              }
-            }
 
-            if (close(write_info.bw_fd) >= 0
-                && write_info.bw_len == 0) {
-              end = 1;                          // success
-            }
-          }
-          close(fd);            // ignore errors for closing read file
+        // copy the file.
+        if (os_copy((char *)backup, (char *)fname, UV_FS_COPYFILE_FICLONE)
+            == 0) {
+          end = 1;  // success
         }
       } else {
         if (vim_rename(backup, fname) == 0) {
diff --git a/src/nvim/lua/vim.lua b/src/nvim/lua/vim.lua
index 38a8795680..9cd8e232d5 100644
--- a/src/nvim/lua/vim.lua
+++ b/src/nvim/lua/vim.lua
@@ -161,8 +161,7 @@ end
 --@returns String with whitespace removed from its beginning and end
 local function trim(s)
   assert(type(s) == 'string', 'Only strings can be trimmed')
-  local result = s:gsub('^%s+', ''):gsub('%s+$', '')
-  return result
+  return s:match('^%s*(.*%S)') or ''
 end
 
 local function __index(t, key)
diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c
index 8d9de1253e..8c7dfcdee7 100644
--- a/src/nvim/os/fs.c
+++ b/src/nvim/os/fs.c
@@ -643,6 +643,21 @@ ptrdiff_t os_write(const int fd, const char *const buf, const size_t size,
   return (ptrdiff_t)written_bytes;
 }
 
+/// Copies a file from `path` to `new_path`.
+///
+/// @see http://docs.libuv.org/en/v1.x/fs.html#c.uv_fs_copyfile
+///
+/// @param path Path of file to be copied
+/// @param path_new Path of new file
+/// @param flags Bitwise OR of flags defined in <uv.h>
+/// @return 0 on success, or libuv error code on failure.
+int os_copy(const char *path, const char *new_path, int flags)
+{
+  int r;
+  RUN_UV_FS_FUNC(r, uv_fs_copyfile, path, new_path, flags, NULL);
+  return r;
+}
+
 /// Flushes file modifications to disk.
 ///
 /// @param fd the file descriptor of the file to flush to disk.
diff --git a/test/functional/core/fileio_spec.lua b/test/functional/core/fileio_spec.lua
index 09533e4e60..c74eb3bb02 100644
--- a/test/functional/core/fileio_spec.lua
+++ b/test/functional/core/fileio_spec.lua
@@ -10,6 +10,8 @@ local request = helpers.request
 local retry = helpers.retry
 local rmdir = helpers.rmdir
 local sleep = helpers.sleep
+local read_file = helpers.read_file
+local trim = helpers.trim
 
 describe('fileio', function()
   before_each(function()
@@ -18,6 +20,7 @@ describe('fileio', function()
     command(':qall!')
     os.remove('Xtest_startup_shada')
     os.remove('Xtest_startup_file1')
+    os.remove('Xtest_startup_file1~')
     os.remove('Xtest_startup_file2')
     rmdir('Xtest_startup_swapdir')
   end)
@@ -64,5 +67,25 @@ describe('fileio', function()
     command('write')
     eq(4, request('nvim__stats').fsync)
   end)
+
+  it('backup #9709', function()
+    clear({ args={ '-i', 'Xtest_startup_shada',
+                   '--cmd', 'set directory=Xtest_startup_swapdir' } })
+
+    command('write Xtest_startup_file1')
+    feed('ifoo<esc>')
+    command('set backup')
+    command('set backupcopy=yes')
+    command('write')
+    feed('Abar<esc>')
+    command('write')
+
+    local foobar_contents = trim(read_file('Xtest_startup_file1'))
+    local bar_contents = trim(read_file('Xtest_startup_file1~'))
+
+    eq('foobar', foobar_contents);
+    eq('foo', bar_contents);
+
+  end)
 end)
 
diff --git a/test/helpers.lua b/test/helpers.lua
index 3311f3ef97..2a6285e685 100644
--- a/test/helpers.lua
+++ b/test/helpers.lua
@@ -697,6 +697,10 @@ local function read_nvim_log()
   return log
 end
 
+local function trim(s)
+  return s:match('^%s*(.*%S)') or ''
+end
+
 local module = {
   REMOVE_THIS = REMOVE_THIS,
   argss_to_cmd = argss_to_cmd,
@@ -736,6 +740,7 @@ local module = {
   updated = updated,
   which = which,
   write_file = write_file,
+  trim = trim,
 }
 module = shared.tbl_extend('error', module, Paths, shared)
 
diff --git a/test/unit/helpers.lua b/test/unit/helpers.lua
index e634b7296e..b5d3dd9f47 100644
--- a/test/unit/helpers.lua
+++ b/test/unit/helpers.lua
@@ -15,6 +15,7 @@ local dedent = global_helpers.dedent
 local neq = global_helpers.neq
 local map = global_helpers.map
 local eq = global_helpers.eq
+local trim = global_helpers.trim
 
 -- C constants.
 local NULL = ffi.cast('void*', 0)
@@ -119,10 +120,6 @@ local deinit = only_separate(function()
   end
 end)
 
-local function trim(s)
-  return s:match('^%s*(.*%S)') or ''
-end
-
 -- a Set that keeps around the lines we've already seen
 local cdefs_init = Set:new()
 local cdefs_mod = nil