file: Add buffered reading and writing

Still no busted tests. Not tested without HAVE_PREADV.

config/CMakeLists.txt

@@ -27,6 +27,7 @@ if(NOT HAVE_SYS_WAIT_H AND UNIX)
 endif()
 check_include_files(sys/utsname.h HAVE_SYS_UTSNAME_H)
 check_include_files(utime.h HAVE_UTIME_H)
+check_include_files(sys/uio.h HAVE_SYS_UIO_H)
 
 # Functions
 check_function_exists(fseeko HAVE_FSEEKO)
@@ -34,6 +35,7 @@ check_function_exists(getpwent HAVE_GETPWENT)
 check_function_exists(getpwnam HAVE_GETPWNAM)
 check_function_exists(getpwuid HAVE_GETPWUID)
 check_function_exists(uv_translate_sys_error HAVE_UV_TRANSLATE_SYS_ERROR)
+check_function_exists(readv HAVE_READV)
 
 if(Iconv_FOUND)
   set(HAVE_ICONV 1)

config/config.h.in

@@ -49,6 +49,13 @@
 #cmakedefine HAVE_WORKING_LIBINTL
 #cmakedefine UNIX
 #cmakedefine USE_FNAME_CASE
+#cmakedefine HAVE_SYS_UIO_H
+#ifdef HAVE_SYS_UIO_H
+#cmakedefine HAVE_READV
+# ifndef HAVE_READV
+#  undef HAVE_SYS_UIO_H
+# endif
+#endif
 
 #cmakedefine FEAT_TUI
 

src/nvim/file.c

@@ -5,26 +5,39 @@
 /// replacement.
 
 #include <unistd.h>
+#include <assert.h>
 #include <stddef.h>
 #include <stdbool.h>
 
+#include "auto/config.h"
+
+#ifdef HAVE_SYS_UIO_H
+# include <sys/uio.h>
+#endif
+
 #include <uv.h>
 
 #include "nvim/file.h"
 #include "nvim/memory.h"
 #include "nvim/os/os.h"
 #include "nvim/globals.h"
+#include "nvim/rbuffer.h"
+#include "nvim/macros.h"
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # include "file.c.generated.h"
 #endif
 
+#define RWBUFSIZE (IOSIZE - 1)
+
 /// Open file
 ///
 /// @param[out]  ret_fp  Address where information needed for reading from or
 ///                      writing to a file is saved
 /// @param[in]  fname  File name to open.
-/// @param[in]  flags  Flags, @see FileOpenFlags.
+/// @param[in]  flags  Flags, @see FileOpenFlags. Currently reading from and
+///                    writing to the file at once is not supported, so either
+///                    FILE_WRITE_ONLY or FILE_READ_ONLY is required.
 /// @param[in]  mode  Permissions for the newly created file (ignored if flags
 ///                   does not have FILE_CREATE\*).
 ///
@@ -41,8 +54,15 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname,
     return fd;
   }
 
+  ret_fp->wr = (bool)(!!(flags & FILE_WRITE_ONLY));
   ret_fp->fd = fd;
   ret_fp->eof = false;
+  ret_fp->rv = rbuffer_new(RWBUFSIZE);
+  ret_fp->_error = 0;
+  if (ret_fp->wr) {
+    ret_fp->rv->data = ret_fp;
+    ret_fp->rv->full_cb = (rbuffer_callback)&file_rb_write_full_cb;
+  }
   return 0;
 }
 
@@ -68,7 +88,7 @@ FileDescriptor *file_open_new(int *const error, const char *const fname,
   return fp;
 }
 
-/// Close file
+/// Close file and free its buffer
 ///
 /// @param[in,out]  fp  File to close.
 ///
@@ -77,6 +97,7 @@ int file_close(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL
 {
   const int error = file_fsync(fp);
   const int error2 = os_close(fp->fd);
+  rbuffer_free(fp->rv);
   if (error2 != 0) {
     return error2;
   }
@@ -103,9 +124,45 @@ int file_free(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL
 int file_fsync(FileDescriptor *const fp)
   FUNC_ATTR_NONNULL_ALL
 {
+  if (!fp->wr) {
+    return 0;
+  }
+  file_rb_write_full_cb(fp->rv, fp);
+  if (fp->_error != 0) {
+    const int error = fp->_error;
+    fp->_error = 0;
+    return error;
+  }
   return os_fsync(fp->fd);
 }
 
+/// Buffer used for writing
+///
+/// Like IObuff, but allows file_\* callers not to care about spoiling it.
+static char writebuf[RWBUFSIZE];
+
+/// Function run when RBuffer is full when writing to a file
+///
+/// Actually does writing to the file, may also be invoked directly.
+///
+/// @param[in,out]  rv  RBuffer instance used.
+/// @param[in,out]  fp  File to work with.
+static void file_rb_write_full_cb(RBuffer *const rv, FileDescriptor *const fp)
+  FUNC_ATTR_NONNULL_ALL
+{
+  assert(fp->wr);
+  assert(rv->data == (void *)fp);
+  const size_t read_bytes = rbuffer_read(rv, writebuf, RWBUFSIZE);
+  const ptrdiff_t wres = os_write(fp->fd, writebuf, read_bytes);
+  if (wres != (ptrdiff_t)read_bytes) {
+    if (wres >= 0) {
+      fp->_error = UV_EIO;
+    } else {
+      fp->_error = (int)wres;
+    }
+  }
+}
+
 /// Read from file
 ///
 /// @param[in,out]  fp  File to work with.
@@ -118,7 +175,69 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf,
                     const size_t size)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  return os_read(fp->fd, &fp->eof, ret_buf, size);
+  assert(!fp->wr);
+  char *buf = ret_buf;
+  size_t read_remaining = size;
+  RBuffer *const rv = fp->rv;
+  while (read_remaining) {
+    const size_t rv_size = rbuffer_size(rv);
+    if (rv_size > 0) {
+      const size_t rsize = rbuffer_read(rv, buf, MIN(rv_size, read_remaining));
+      buf += rsize;
+      read_remaining -= rsize;
+    }
+    if (fp->eof) {
+      break;
+    }
+    if (read_remaining) {
+      assert(rbuffer_size(rv) == 0);
+      rbuffer_reset(rv);
+      if (read_remaining >= RWBUFSIZE) {
+#ifdef HAVE_READV
+        // If there is readv() syscall, then take an opportunity to populate
+        // both target buffer and RBuffer at once, …
+        size_t read_count;
+        struct iovec iov[] = {
+          { .iov_base = buf, .iov_len = read_remaining },
+          { .iov_base = rbuffer_read_ptr(rv, &read_count), .iov_len = RWBUFSIZE
+          },
+        };
+        assert(read_count == RWBUFSIZE);
+        const ptrdiff_t r_ret = os_readv(fp->fd, &fp->eof, iov,
+                                         ARRAY_SIZE(iov));
+        if (r_ret > 0) {
+          if (r_ret > (ptrdiff_t)read_remaining) {
+            rbuffer_produced(rv, (size_t)(r_ret - (ptrdiff_t)read_remaining));
+          }
+        } else if (r_ret < 0) {
+          return r_ret;
+        }
+#else
+        // …otherwise leave RBuffer empty and populate only target buffer,
+        // because filtering information through rbuffer will be more syscalls.
+        const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof, buf, read_remaining);
+        if (r_ret >= 0) {
+          read_remaining -= (size_t)r_ret;
+          return (ptrdiff_t)(size - read_remaining);
+        } else if (r_ret < 0) {
+          return r_ret;
+        }
+#endif
+      } else {
+        size_t write_count;
+        const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof,
+                                        rbuffer_write_ptr(rv, &write_count),
+                                        RWBUFSIZE);
+        assert(write_count == RWBUFSIZE);
+        if (r_ret > 0) {
+          rbuffer_produced(rv, (size_t)r_ret);
+        } else if (r_ret < 0) {
+          return r_ret;
+        }
+      }
+    }
+  }
+  return (ptrdiff_t)(size - read_remaining);
 }
 
 /// Write to a file
@@ -132,12 +251,21 @@ ptrdiff_t file_write(FileDescriptor *const fp, const char *const buf,
                      const size_t size)
   FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ARG(1)
 {
-  return os_write(fp->fd, buf, size);
+  assert(fp->wr);
+  const size_t written = rbuffer_write(fp->rv, buf, size);
+  if (fp->_error != 0) {
+    const int error = fp->_error;
+    fp->_error = 0;
+    return error;
+  } else if (written != size) {
+    return UV_EIO;
+  }
+  return (ptrdiff_t)written;
 }
 
 /// Buffer used for skipping. Its contents is undefined and should never be
 /// used.
-static char skipbuf[IOSIZE];
+static char skipbuf[RWBUFSIZE];
 
 /// Skip some bytes
 ///
@@ -146,6 +274,7 @@ static char skipbuf[IOSIZE];
 ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size)
   FUNC_ATTR_NONNULL_ALL
 {
+  assert(!fp->wr);
   size_t read_bytes = 0;
   do {
     ptrdiff_t new_read_bytes = file_read(

src/nvim/file.h

@@ -6,10 +6,14 @@
 #include <fcntl.h>
 
 #include "nvim/func_attr.h"
+#include "nvim/rbuffer.h"
 
 /// Structure used to read from/write to file
 typedef struct {
   int fd;  ///< File descriptor.
+  int _error;  ///< Error code for use with RBuffer callbacks or zero.
+  RBuffer *rv;  ///< Read or write buffer.
+  bool wr;  ///< True if file is in write mode.
   bool eof;  ///< True if end of file was encountered.
 } FileDescriptor;
 

src/nvim/os/fs.c

@@ -7,6 +7,12 @@
 #include <fcntl.h>
 #include <errno.h>
 
+#include "auto/config.h"
+
+#ifdef HAVE_SYS_UIO_H
+# include <sys/uio.h>
+#endif
+
 #include <uv.h>
 
 #include "nvim/os/os.h"
@@ -417,6 +423,73 @@ ptrdiff_t os_read(const int fd, bool *ret_eof, char *const ret_buf,
   return (ptrdiff_t) read_bytes;
 }
 
+#ifdef HAVE_READV
+/// Read from a file to a multiple buffers at once
+///
+/// Wrapper for readv().
+///
+/// @param[in]  fd  File descriptor to read from.
+/// @param[out]  ret_eof  Is set to true if EOF was encountered, otherwise set
+///                       to false. Initial value is ignored.
+/// @param[out]  iov  Description of buffers to write to. Note: this description
+///                   may change, it is incorrect to use data it points to after
+///                   os_readv().
+/// @param[in]  iov_size  Number of buffers in iov.
+ptrdiff_t os_readv(int fd, bool *ret_eof, struct iovec *iov, size_t iov_size)
+  FUNC_ATTR_NONNULL_ALL
+{
+  *ret_eof = false;
+  size_t read_bytes = 0;
+  bool did_try_to_free = false;
+  size_t toread = 0;
+  for (size_t i = 0; i < iov_size; i++) {
+    // Overflow, trying to read too much data
+    assert(toread <= SIZE_MAX - iov[i].iov_len);
+    toread += iov[i].iov_len;
+  }
+  while (read_bytes < toread && iov_size && !*ret_eof) {
+    ptrdiff_t cur_read_bytes = readv(fd, iov, (int)iov_size);
+    if (toread && cur_read_bytes == 0) {
+      *ret_eof = true;
+    }
+    if (cur_read_bytes > 0) {
+      read_bytes += (size_t)cur_read_bytes;
+      while (iov_size && cur_read_bytes) {
+        if (cur_read_bytes < (ptrdiff_t) iov->iov_len) {
+          iov->iov_len -= (size_t)cur_read_bytes;
+          iov->iov_base = (char *)iov->iov_base + cur_read_bytes;
+          cur_read_bytes = 0;
+        } else {
+          cur_read_bytes -= (ptrdiff_t)iov->iov_len;
+          iov_size--;
+          iov++;
+        }
+      }
+    } else if (cur_read_bytes < 0) {
+#ifdef HAVE_UV_TRANSLATE_SYS_ERROR
+      const int error = uv_translate_sys_error(errno);
+#else
+      const int error = -errno;
+      STATIC_ASSERT(-EINTR == UV_EINTR, "Need to translate error codes");
+      STATIC_ASSERT(-EAGAIN == UV_EAGAIN, "Need to translate error codes");
+      STATIC_ASSERT(-ENOMEM == UV_ENOMEM, "Need to translate error codes");
+#endif
+      errno = 0;
+      if (error == UV_EINTR || error == UV_EAGAIN) {
+        continue;
+      } else if (error == UV_ENOMEM && !did_try_to_free) {
+        try_to_free_memory();
+        did_try_to_free = true;
+        continue;
+      } else {
+        return (ptrdiff_t)error;
+      }
+    }
+  }
+  return (ptrdiff_t)read_bytes;
+}
+#endif  // HAVE_READV
+
 /// Write to a file
 ///
 /// @param[in]  fd  File descriptor to write to.

src/nvim/rbuffer.c

@@ -153,7 +153,7 @@ void rbuffer_consumed(RBuffer *buf, size_t count)
 
 // Higher level functions for copying from/to RBuffer instances and data
 // pointers
-size_t rbuffer_write(RBuffer *buf, char *src, size_t src_size)
+size_t rbuffer_write(RBuffer *buf, const char *src, size_t src_size)
   FUNC_ATTR_NONNULL_ALL
 {
   size_t size = src_size;

src/nvim/shada.c

@@ -715,11 +715,6 @@ static void close_sd_reader(ShaDaReadDef *const sd_reader)
 static void close_sd_writer(ShaDaWriteDef *const sd_writer)
   FUNC_ATTR_NONNULL_ALL
 {
-  const int error = file_fsync(sd_writer->cookie);
-  if (error < 0) {
-    emsgf(_(SERR "System error while synchronizing ShaDa file: %s"),
-          os_strerror(error));
-  }
   close_file(sd_writer->cookie);
 }
 
@@ -2995,15 +2990,11 @@ shada_write_file_nomerge: {}
                                                            ? NULL
                                                            : &sd_reader));
   assert(sw_ret != kSDWriteIgnError);
-#ifndef UNIX
-  sd_writer.close(&sd_writer);
-#endif
   if (!nomerge) {
     sd_reader.close(&sd_reader);
     bool did_remove = false;
     if (sw_ret == kSDWriteSuccessfull) {
 #ifdef UNIX
-      bool closed = false;
       // For Unix we check the owner of the file.  It's not very nice to
       // overwrite a user’s viminfo file after a "su root", with a
       // viminfo file that the user can't read.
@@ -3016,13 +3007,11 @@ shada_write_file_nomerge: {}
             const uv_gid_t old_gid = (uv_gid_t) old_info.stat.st_gid;
             const int fchown_ret = os_fchown(file_fd(sd_writer.cookie),
                                              old_uid, old_gid);
-            sd_writer.close(&sd_writer);
             if (fchown_ret != 0) {
               EMSG3(_(RNERR "Failed setting uid and gid for file %s: %s"),
                     tempname, os_strerror(fchown_ret));
               goto shada_write_file_did_not_remove;
             }
-            closed = true;
           }
         } else if (!(old_info.stat.st_uid == getuid()
                      ? (old_info.stat.st_mode & 0200)
@@ -3030,13 +3019,9 @@ shada_write_file_nomerge: {}
                         ? (old_info.stat.st_mode & 0020)
                         : (old_info.stat.st_mode & 0002)))) {
           EMSG2(_("E137: ShaDa file is not writable: %s"), fname);
-          sd_writer.close(&sd_writer);
           goto shada_write_file_did_not_remove;
         }
       }
-      if (!closed) {
-        sd_writer.close(&sd_writer);
-      }
 #endif
       if (vim_rename(tempname, fname) == -1) {
         EMSG3(_(RNERR "Can't rename ShaDa file from %s to %s!"),
@@ -3063,6 +3048,7 @@ shada_write_file_did_not_remove:
     }
     xfree(tempname);
   }
+  sd_writer.close(&sd_writer);
 
   xfree(fname);
   return OK;
@@ -3192,20 +3178,20 @@ static ShaDaReadResult fread_len(ShaDaReadDef *const sd_reader,
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
   const ptrdiff_t read_bytes = sd_reader->read(sd_reader, buffer, length);
-  (void) read_bytes;
 
-  if (sd_reader->error != NULL) {
+  if (read_bytes != (ptrdiff_t)length) {
-    emsgf(_(SERR "System error while reading ShaDa file: %s"),
+    if (sd_reader->error != NULL) {
-          sd_reader->error);
+      emsgf(_(SERR "System error while reading ShaDa file: %s"),
-    return kSDReadStatusReadError;
+            sd_reader->error);
-  } else if (sd_reader->eof) {
+      return kSDReadStatusReadError;
-    emsgf(_(RCERR "Error while reading ShaDa file: "
+    } else {
-            "last entry specified that it occupies %" PRIu64 " bytes, "
+      emsgf(_(RCERR "Error while reading ShaDa file: "
-            "but file ended earlier"),
+              "last entry specified that it occupies %" PRIu64 " bytes, "
-          (uint64_t) length);
+              "but file ended earlier"),
-    return kSDReadStatusNotShaDa;
+            (uint64_t) length);
+      return kSDReadStatusNotShaDa;
+    }
   }
-  assert(read_bytes >= 0 && (size_t) read_bytes == length);
   return kSDReadStatusSuccess;
 }