Merge #6111 from ZyX-I/split-eval'/os-fileio

Refactor writefile() and create more tests for it

5 files changed, 92 insertions, 37 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 49644d70ef..a582c56208 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -30,6 +30,7 @@
 #include "nvim/ex_eval.h"
 #include "nvim/ex_getln.h"
 #include "nvim/fileio.h"
+#include "nvim/os/fileio.h"
 #include "nvim/func_attr.h"
 #include "nvim/fold.h"
 #include "nvim/getchar.h"
@@ -18467,29 +18468,60 @@ static void f_winsaveview(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 }
 
 /// Writes list of strings to file
-static bool write_list(FILE *fd, list_T *list, bool binary)
-{
-  int ret = true;
-
-  for (listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
-    for (char_u *s = get_tv_string(&li->li_tv); *s != NUL; ++s) {
-      if (putc(*s == '\n' ? NUL : *s, fd) == EOF) {
-        ret = false;
-        break;
+///
+/// @param  fp  File to write to.
+/// @param[in]  list  List to write.
+/// @param[in]  binary  Whether to write in binary mode.
+///
+/// @return true in case of success, false otherwise.
+static bool write_list(FileDescriptor *const fp, const list_T *const list,
+                       const bool binary)
+{
+  int error = 0;
+  for (const listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
+    const char *const s = (const char *)get_tv_string_chk(
+        (typval_T *)&li->li_tv);
+    if (s == NULL) {
+      return false;
+    }
+    const char *hunk_start = s;
+    for (const char *p = hunk_start;; p++) {
+      if (*p == NUL || *p == NL) {
+        if (p != hunk_start) {
+          const ptrdiff_t written = file_write(fp, hunk_start,
+                                               (size_t)(p - hunk_start));
+          if (written < 0) {
+            error = (int)written;
+            goto write_list_error;
+          }
+        }
+        if (*p == NUL) {
+          break;
+        } else {
+          hunk_start = p + 1;
+          const ptrdiff_t written = file_write(fp, (char[]){ NUL }, 1);
+          if (written < 0) {
+            error = (int)written;
+            break;
+          }
+        }
       }
     }
     if (!binary || li->li_next != NULL) {
-      if (putc('\n', fd) == EOF) {
-        ret = false;
-        break;
+      const ptrdiff_t written = file_write(fp, "\n", 1);
+      if (written < 0) {
+        error = (int)written;
+        goto write_list_error;
       }
     }
-    if (ret == false) {
-      EMSG(_(e_write));
-      break;
-    }
   }
-  return ret;
+  if ((error = file_fsync(fp)) != 0) {
+    goto write_list_error;
+  }
+  return true;
+write_list_error:
+  emsgf(_("E80: Error while writing: %s"), os_strerror(error));
+  return false;
 }
 
 /// Initializes a static list with 10 items.
@@ -18617,27 +18649,42 @@ static void f_writefile(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   bool binary = false;
   bool append = false;
   if (argvars[2].v_type != VAR_UNKNOWN) {
-    if (vim_strchr(get_tv_string(&argvars[2]), 'b')) {
+    const char *const flags = (const char *)get_tv_string_chk(&argvars[2]);
+    if (flags == NULL) {
+      return;
+    }
+    if (strchr(flags, 'b')) {
       binary = true;
     }
-    if (vim_strchr(get_tv_string(&argvars[2]), 'a')) {
+    if (strchr(flags, 'a')) {
       append = true;
     }
   }
 
-  // Always open the file in binary mode, library functions have a mind of
-  // their own about CR-LF conversion.
-  char_u *fname = get_tv_string(&argvars[1]);
-  FILE *fd;
-  if (*fname == NUL || (fd = mch_fopen((char *)fname,
-       append ? APPENDBIN : WRITEBIN)) == NULL) {
-    EMSG2(_(e_notcreate), *fname == NUL ? (char_u *)_("<empty>") : fname);
-    rettv->vval.v_number = -1;
+  const char buf[NUMBUFLEN];
+  const char *const fname = (const char *)get_tv_string_buf_chk(&argvars[1],
+                                                                (char_u *)buf);
+  if (fname == NULL) {
+    return;
+  }
+  FileDescriptor *fp;
+  int error;
+  rettv->vval.v_number = -1;
+  if (*fname == NUL) {
+    EMSG(_("E482: Can't open file with an empty name"));
+  } else if ((fp = file_open_new(&error, fname,
+                                 ((append ? kFileAppend : kFileTruncate)
+                                  | kFileCreate), 0666)) == NULL) {
+    emsgf(_("E482: Can't open file %s for writing: %s"),
+          fname, os_strerror(error));
   } else {
-    if (write_list(fd, argvars[0].vval.v_list, binary) == false) {
-      rettv->vval.v_number = -1;
+    if (write_list(fp, argvars[0].vval.v_list, binary)) {
+      rettv->vval.v_number = 0;
+    }
+    if ((error = file_free(fp)) != 0) {
+      emsgf(_("E80: Error when closing file %s: %s"),
+            fname, os_strerror(error));
     }
-    fclose(fd);
   }
 }
 /*
diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
index a03878dec2..0fda9a8ae6 100644
--- a/src/nvim/ex_cmds.c
+++ b/src/nvim/ex_cmds.c
@@ -1131,11 +1131,12 @@ static void do_filter(
    */
   ++no_wait_return;             /* don't call wait_return() while busy */
   if (itmp != NULL && buf_write(curbuf, itmp, NULL, line1, line2, eap,
-          FALSE, FALSE, FALSE, TRUE) == FAIL) {
-    msg_putchar('\n');                  /* keep message from buf_write() */
-    --no_wait_return;
-    if (!aborting())
-      (void)EMSG2(_(e_notcreate), itmp);        /* will call wait_return */
+                                false, false, false, true) == FAIL) {
+    msg_putchar('\n');  // Keep message from buf_write().
+    no_wait_return--;
+    if (!aborting()) {
+      EMSG2(_("E482: Can't create file %s"), itmp);  // Will call wait_return.
+    }
     goto filterend;
   }
   if (curbuf != old_curbuf)
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
index b141d40aa2..ba4f6e2b3b 100644
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -1150,7 +1150,6 @@ EXTERN char_u e_noprev[] INIT(= N_("E34: No previous command"));
 EXTERN char_u e_noprevre[] INIT(= N_("E35: No previous regular expression"));
 EXTERN char_u e_norange[] INIT(= N_("E481: No range allowed"));
 EXTERN char_u e_noroom[] INIT(= N_("E36: Not enough room"));
-EXTERN char_u e_notcreate[] INIT(= N_("E482: Can't create file %s"));
 EXTERN char_u e_notmp[] INIT(= N_("E483: Can't get temp file name"));
 EXTERN char_u e_notopen[] INIT(= N_("E484: Can't open file %s"));
 EXTERN char_u e_notread[] INIT(= N_("E485: Can't read file %s"));
diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c
index cf5bfd60ae..775f2bd449 100644
--- a/src/nvim/os/fileio.c
+++ b/src/nvim/os/fileio.c
@@ -62,6 +62,8 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname,
   FLAG(flags, kFileCreate, O_CREAT|O_WRONLY, kTrue, !(flags & kFileCreateOnly));
   FLAG(flags, kFileTruncate, O_TRUNC|O_WRONLY, kTrue,
        !(flags & kFileCreateOnly));
+  FLAG(flags, kFileAppend, O_APPEND|O_WRONLY, kTrue,
+       !(flags & kFileCreateOnly));
   FLAG(flags, kFileReadOnly, O_RDONLY, kFalse, wr != kTrue);
 #ifdef O_NOFOLLOW
   FLAG(flags, kFileNoSymlink, O_NOFOLLOW, kNone, true);
@@ -153,7 +155,11 @@ int file_fsync(FileDescriptor *const fp)
     fp->_error = 0;
     return error;
   }
-  return os_fsync(fp->fd);
+  const int error = os_fsync(fp->fd);
+  if (error != UV_EINVAL && error != UV_EROFS) {
+    return error;
+  }
+  return 0;
 }
 
 /// Buffer used for writing
diff --git a/src/nvim/os/fileio.h b/src/nvim/os/fileio.h
index 2cffd5c851..0b55cc695f 100644
--- a/src/nvim/os/fileio.h
+++ b/src/nvim/os/fileio.h
@@ -30,6 +30,8 @@ typedef enum {
   kFileTruncate = 32,  ///< Truncate the file if it exists.
                        ///< Implies kFileWriteOnly. Cannot be used with
                        ///< kFileCreateOnly.
+  kFileAppend = 64,  ///< Append to the file. Implies kFileWriteOnly. Cannot
+                     ///< be used with kFileCreateOnly.
 } FileOpenFlags;
 
 static inline bool file_eof(const FileDescriptor *const fp)