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

ref #8288

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) {
+    if (backup_copy) {
-      int bfd;
+      char_u *wp;
-      char_u      *copybuf, *wp;
+      int some_error = false;
-      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, ",");
+        (void)copy_option_part(&dirp, IObuff, IOSIZE, ",");
-        rootname = get_file_in_dir(fname, copybuf);
+        rootname = get_file_in_dir(fname, IObuff);
         if (rootname == NULL) {
           some_error = TRUE;                /* out of memory */
           goto nobackup;
@@ -2875,68 +2867,14 @@ 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
+          // copy the file
-            /*
+          if (os_copy((char *)fname, (char *)backup, UV_FS_COPYFILE_FICLONE)
-             * Try to set the group of the backup same as the
+              != 0) {
-             * original file. If this fails, set the protection
+            SET_ERRMSG(_("E506: Can't write to backup file "
-             * bits for the group same as the protection bits for
+                         "(add ! to override)"));
-             * 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;
-              }
           }
 
-            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,
@@ -2951,11 +2889,8 @@ buf_write (
           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,29 +3472,12 @@ 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
+        // copy the file.
-                && write_info.bw_len == 0) {
+        if (os_copy((char *)backup, (char *)fname, UV_FS_COPYFILE_FICLONE)
+            == 0) {
           end = 1;  // success
         }
-          }
-          close(fd);            // ignore errors for closing read file
-        }
       } else {
         if (vim_rename(backup, fname) == 0) {
           end = 1;

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 s:match('^%s*(.*%S)') or ''
-  return result
 end
 
 local function __index(t, key)

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.

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)
 

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)
 

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