eval: Make writefile() able to disable fsync()

5 files changed, 66 insertions, 33 deletions

diff --git a/runtime/doc/eval.txt b/runtime/doc/eval.txt
index 9a86e13d95..727199f742 100644
--- a/runtime/doc/eval.txt
+++ b/runtime/doc/eval.txt
@@ -7915,6 +7915,11 @@ writefile({list}, {fname} [, {flags}])
 		appended to the file: >
 			:call writefile(["foo"], "event.log", "a")
 			:call writefile(["bar"], "event.log", "a")
+<
+		When {flags} contains "S" fsync() call is not used. This means 
+		that writefile() will finish faster, but writes may be left in 
+		OS buffers and not yet written to disk. Such changes will 
+		disappear if system crashes before OS does writing.
 
 		All NL characters are replaced with a NUL character.
 		Inserting CR characters needs to be done before passing {list}
diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index f0d78a2508..8a3e3f3e22 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -17421,6 +17421,7 @@ static void f_writefile(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 
   bool binary = false;
   bool append = false;
+  bool do_fsync = true;
   if (argvars[2].v_type != VAR_UNKNOWN) {
     const char *const flags = tv_get_string_chk(&argvars[2]);
     if (flags == NULL) {
@@ -17432,6 +17433,9 @@ static void f_writefile(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     if (strchr(flags, 'a')) {
       append = true;
     }
+    if (strchr(flags, 'S')) {
+      do_fsync = false;
+    }
   }
 
   char buf[NUMBUFLEN];
@@ -17453,7 +17457,7 @@ static void f_writefile(typval_T *argvars, typval_T *rettv, FunPtr fptr)
     if (write_list(&fp, argvars[0].vval.v_list, binary)) {
       rettv->vval.v_number = 0;
     }
-    if ((error = file_close(&fp)) != 0) {
+    if ((error = file_close(&fp, do_fsync)) != 0) {
       emsgf(_("E80: Error when closing file %s: %s"),
             fname, os_strerror(error));
     }
diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c
index 3c928363cc..4b7b53fc7f 100644
--- a/src/nvim/os/fileio.c
+++ b/src/nvim/os/fileio.c
@@ -113,27 +113,31 @@ FileDescriptor *file_open_new(int *const error, const char *const fname,
 /// Close file and free its buffer
 ///
 /// @param[in,out]  fp  File to close.
+/// @param[in]  do_fsync  If true, use fsync() to write changes to disk.
 ///
 /// @return 0 or error code.
-int file_close(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL
+int file_close(FileDescriptor *const fp, const bool do_fsync)
+  FUNC_ATTR_NONNULL_ALL
 {
-  const int error = file_fsync(fp);
-  const int error2 = os_close(fp->fd);
+  const int flush_error = (do_fsync ? file_fsync(fp) : file_flush(fp));
+  const int close_error = os_close(fp->fd);
   rbuffer_free(fp->rv);
-  if (error2 != 0) {
-    return error2;
+  if (close_error != 0) {
+    return close_error;
   }
-  return error;
+  return flush_error;
 }
 
 /// Close and free file obtained using file_open_new()
 ///
 /// @param[in,out]  fp  File to close.
+/// @param[in]  do_fsync  If true, use fsync() to write changes to disk.
 ///
 /// @return 0 or error code.
-int file_free(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL
+int file_free(FileDescriptor *const fp, const bool do_fsync)
+  FUNC_ATTR_NONNULL_ALL
 {
-  const int ret = file_close(fp);
+  const int ret = file_close(fp, do_fsync);
   xfree(fp);
   return ret;
 }
diff --git a/src/nvim/shada.c b/src/nvim/shada.c
index f65fdaf1c0..c7b95958e0 100644
--- a/src/nvim/shada.c
+++ b/src/nvim/shada.c
@@ -811,7 +811,7 @@ static int open_shada_file_for_reading(const char *const fname,
 /// Wrapper for closing file descriptors
 static void close_file(void *cookie)
 {
-  const int error = file_free(cookie);
+  const int error = file_free(cookie, true);
   if (error != 0) {
     emsgf(_(SERR "System error while closing ShaDa file: %s"),
           os_strerror(error));
diff --git a/test/unit/os/fileio_spec.lua b/test/unit/os/fileio_spec.lua
index 5e1b2523fa..6c1ae73847 100644
--- a/test/unit/os/fileio_spec.lua
+++ b/test/unit/os/fileio_spec.lua
@@ -98,7 +98,7 @@ describe('file_open', function()
     eq(0, err)
     local attrs = lfs.attributes(filec)
     eq('rwx------', attrs.permissions)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can create a rw------- file with kFileCreate', function()
@@ -106,7 +106,7 @@ describe('file_open', function()
     eq(0, err)
     local attrs = lfs.attributes(filec)
     eq('rw-------', attrs.permissions)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can create a rwx------ file with kFileCreateOnly', function()
@@ -114,7 +114,7 @@ describe('file_open', function()
     eq(0, err)
     local attrs = lfs.attributes(filec)
     eq('rwx------', attrs.permissions)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can create a rw------- file with kFileCreateOnly', function()
@@ -122,7 +122,7 @@ describe('file_open', function()
     eq(0, err)
     local attrs = lfs.attributes(filec)
     eq('rw-------', attrs.permissions)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('fails to open an existing file with kFileCreateOnly', function()
@@ -141,35 +141,35 @@ describe('file_open', function()
     local err, fp = file_open(file1, m.kFileCreate, 384)
     eq(0, err)
     eq(true, fp.wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can open an existing file read-only with zero', function()
     local err, fp = file_open(file1, 0, 384)
     eq(0, err)
     eq(false, fp.wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can open an existing file read-only with kFileReadOnly', function()
     local err, fp = file_open(file1, m.kFileReadOnly, 384)
     eq(0, err)
     eq(false, fp.wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can open an existing file read-only with kFileNoSymlink', function()
     local err, fp = file_open(file1, m.kFileNoSymlink, 384)
     eq(0, err)
     eq(false, fp.wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can truncate an existing file with kFileTruncate', function()
     local err, fp = file_open(file1, m.kFileTruncate, 384)
     eq(0, err)
     eq(true, fp.wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
     local attrs = lfs.attributes(file1)
     eq(0, attrs.size)
   end)
@@ -178,7 +178,7 @@ describe('file_open', function()
     local err, fp = file_open(file1, m.kFileWriteOnly, 384)
     eq(0, err)
     eq(true, fp.wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
     local attrs = lfs.attributes(file1)
     eq(4096, attrs.size)
   end)
@@ -195,7 +195,7 @@ describe('file_open', function()
     local err, fp = file_open(linkf, m.kFileTruncate, 384)
     eq(0, err)
     eq(true, fp.wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
     local attrs = lfs.attributes(file1)
     eq(0, attrs.size)
   end)
@@ -221,7 +221,7 @@ describe('file_open_new', function()
     local err, fp = file_open_new(file1, 0, 384)
     eq(0, err)
     eq(false, fp.wr)
-    eq(0, m.file_free(fp))
+    eq(0, m.file_free(fp, false))
   end)
 
   itp('fails to open an existing file with kFileCreateOnly', function()
@@ -231,7 +231,27 @@ describe('file_open_new', function()
   end)
 end)
 
--- file_close is called above, so it is not tested directly
+describe('file_close', function()
+  itp('can flush writes to disk also with true argument', function()
+    local err, fp = file_open(filec, m.kFileCreateOnly, 384)
+    local wsize = file_write(fp, 'test')
+    eq(4, wsize)
+    eq(0, lfs.attributes(filec).size)
+    eq(0, m.file_close(fp, true))
+    eq(wsize, lfs.attributes(filec).size)
+  end)
+end)
+
+describe('file_free', function()
+  itp('can flush writes to disk also with true argument', function()
+    local err, fp = file_open_new(filec, m.kFileCreateOnly, 384)
+    local wsize = file_write(fp, 'test')
+    eq(4, wsize)
+    eq(0, lfs.attributes(filec).size)
+    eq(0, m.file_free(fp, true))
+    eq(wsize, lfs.attributes(filec).size)
+  end)
+end)
 
 describe('file_fsync', function()
   itp('can flush writes to disk', function()
@@ -244,7 +264,7 @@ describe('file_fsync', function()
     eq(0, lfs.attributes(filec).size)
     eq(0, file_fsync(fp))
     eq(wsize, lfs.attributes(filec).size)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 end)
 
@@ -259,7 +279,7 @@ describe('file_flush', function()
     eq(0, lfs.attributes(filec).size)
     eq(0, file_flush(fp))
     eq(wsize, lfs.attributes(filec).size)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 end)
 
@@ -281,7 +301,7 @@ describe('file_read', function()
       eq({exp_err, exp_s}, {file_read(fp, size)})
       shift = shift + size
     end
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can read the whole file at once', function()
@@ -290,7 +310,7 @@ describe('file_read', function()
     eq(false, fp.wr)
     eq({#fcontents, fcontents}, {file_read(fp, #fcontents)})
     eq({0, ('\0'):rep(#fcontents)}, {file_read(fp, #fcontents)})
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can read more then 1024 bytes after reading a small chunk', function()
@@ -300,7 +320,7 @@ describe('file_read', function()
     eq({5, fcontents:sub(1, 5)}, {file_read(fp, 5)})
     eq({#fcontents - 5, fcontents:sub(6) .. (('\0'):rep(5))},
        {file_read(fp, #fcontents)})
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 
   itp('can read file by 768-byte-chunks', function()
@@ -320,7 +340,7 @@ describe('file_read', function()
       eq({exp_err, exp_s}, {file_read(fp, size)})
       shift = shift + size
     end
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 end)
 
@@ -331,7 +351,7 @@ describe('file_write', function()
     eq(true, fp.wr)
     local wr = file_write(fp, fcontents)
     eq(#fcontents, wr)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
     eq(wr, lfs.attributes(filec).size)
     eq(fcontents, io.open(filec):read('*a'))
   end)
@@ -348,7 +368,7 @@ describe('file_write', function()
       eq(wr, #s)
       shift = shift + size
     end
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
     eq(#fcontents, lfs.attributes(filec).size)
     eq(fcontents, io.open(filec):read('*a'))
   end)
@@ -365,7 +385,7 @@ describe('file_write', function()
       eq(wr, #s)
       shift = shift + size
     end
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
     eq(#fcontents, lfs.attributes(filec).size)
     eq(fcontents, io.open(filec):read('*a'))
   end)
@@ -380,6 +400,6 @@ describe('file_skip', function()
     local rd, s = file_read(fp, 3)
     eq(3, rd)
     eq(fcontents:sub(4, 6), s)
-    eq(0, m.file_close(fp))
+    eq(0, m.file_close(fp, false))
   end)
 end)