From 11dda658d6f0c4470a54012df71be73b4e9a5f57 Mon Sep 17 00:00:00 2001 From: ZyX Date: Wed, 1 Jun 2016 22:57:52 +0300 Subject: file,os/fs,shada: Separate opening, closing, writing and reading files Moves low-level functions handling to os/fs.c. Adds file.c with a proxy interface. Target: while leaving syscalls handling is os.c (partially handled by libuv), add buffering for reading and writing to file.c. --- src/nvim/file.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/nvim/file.h | 59 +++++++++++++++ src/nvim/os/fs.c | 213 +++++++++++++++++++++++++++++++++++++++------------- src/nvim/shada.c | 224 +++++++++++++++++++------------------------------------ 4 files changed, 461 insertions(+), 199 deletions(-) create mode 100644 src/nvim/file.c create mode 100644 src/nvim/file.h (limited to 'src') diff --git a/src/nvim/file.c b/src/nvim/file.c new file mode 100644 index 0000000000..bc230ecf00 --- /dev/null +++ b/src/nvim/file.c @@ -0,0 +1,164 @@ +/// @file file.c +/// +/// Buffered reading/writing to a file. Unlike fileio.c this is not dealing with +/// Neovim stuctures for buffer, with autocommands, etc: just fopen/fread/fwrite +/// replacement. + +#include +#include +#include + +#include + +#include "nvim/file.h" +#include "nvim/memory.h" +#include "nvim/os/os.h" +#include "nvim/globals.h" + +#ifdef INCLUDE_GENERATED_DECLARATIONS +# include "file.c.generated.h" +#endif + +/// 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] mode Permissions for the newly created file (ignored if flags +/// does not have FILE_CREATE\*). +/// +/// @return Error code (@see os_strerror()) or 0. +int file_open(FileDescriptor *const ret_fp, const char *const fname, + const int flags, const int mode) + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT +{ + int fd; + + fd = os_open(fname, flags, mode); + + if (fd < 0) { + return fd; + } + + ret_fp->fd = fd; + ret_fp->eof = false; + return 0; +} + +/// Like file_open(), but allocate and return ret_fp +/// +/// @param[out] error Error code, @see os_strerror(). Is set to zero on +/// success. +/// @param[in] fname File name to open. +/// @param[in] flags Flags, @see FileOpenFlags. +/// @param[in] mode Permissions for the newly created file (ignored if flags +/// does not have FILE_CREATE\*). +/// +/// @return [allocated] Opened file or NULL in case of error. +FileDescriptor *file_open_new(int *const error, const char *const fname, + const int flags, const int mode) + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT +{ + FileDescriptor *const fp = xmalloc(sizeof(*fp)); + if ((*error = file_open(fp, fname, flags, mode)) != 0) { + xfree(fp); + return NULL; + } + return fp; +} + +/// Close file +/// +/// @param[in,out] fp File to close. +/// +/// @return 0 or error code. +int file_close(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL +{ + const int error = file_fsync(fp); + const int error2 = os_close(fp->fd); + if (error2 != 0) { + return error2; + } + return error; +} + +/// Close and free file obtained using file_open_new() +/// +/// @param[in,out] fp File to close. +/// +/// @return 0 or error code. +int file_free(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL +{ + const int ret = file_close(fp); + xfree(fp); + return ret; +} + +/// Flush file modifications to disk +/// +/// @param[in,out] fp File to work with. +/// +/// @return 0 or error code. +int file_fsync(FileDescriptor *const fp) + FUNC_ATTR_NONNULL_ALL +{ + return os_fsync(fp->fd); +} + +/// Read from file +/// +/// @param[in,out] fp File to work with. +/// @param[out] ret_buf Buffer to read to. Must not be NULL. +/// @param[in] size Number of bytes to read. Buffer must have at least ret_buf +/// bytes. +/// +/// @return error_code (< 0) or number of bytes read. +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); +} + +/// Write to a file +/// +/// @param[in] fd File descriptor to write to. +/// @param[in] buf Data to write. May be NULL if size is zero. +/// @param[in] size Amount of bytes to write. +/// +/// @return Number of bytes written or libuv error code (< 0). +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); +} + +/// Buffer used for skipping. Its contents is undefined and should never be +/// used. +static char skipbuf[IOSIZE]; + +/// Skip some bytes +/// +/// This is like `fseek(fp, size, SEEK_CUR)`, but actual implementation simply +/// reads to a buffer and discards the result. +ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size) + FUNC_ATTR_NONNULL_ALL +{ + size_t read_bytes = 0; + do { + ptrdiff_t new_read_bytes = file_read( + fp, skipbuf, (size_t)(size - read_bytes > sizeof(skipbuf) + ? sizeof(skipbuf) + : size - read_bytes)); + if (new_read_bytes < 0) { + return new_read_bytes; + } else if (new_read_bytes == 0) { + break; + } + read_bytes += (size_t)new_read_bytes; + } while (read_bytes < size && !fp->eof); + + return (ptrdiff_t)read_bytes; +} diff --git a/src/nvim/file.h b/src/nvim/file.h new file mode 100644 index 0000000000..0aa98e0def --- /dev/null +++ b/src/nvim/file.h @@ -0,0 +1,59 @@ +#ifndef NVIM_FILE_H +#define NVIM_FILE_H + +#include +#include +#include + +#include "nvim/func_attr.h" + +/// Structure used to read from/write to file +typedef struct { + int fd; ///< File descriptor. + bool eof; ///< True if end of file was encountered. +} FileDescriptor; + +/// file_open() flags +typedef enum { + FILE_READ_ONLY = O_RDONLY, ///< Open file read-only. + FILE_CREATE = O_CREAT, ///< Create file if it does not exist yet. + FILE_WRITE_ONLY = O_WRONLY, ///< Open file for writing only. +#ifdef O_NOFOLLOW + FILE_NOSYMLINK = O_NOFOLLOW, ///< Do not allow symbolic links. +#else + FILE_NOSYMLINK = 0, +#endif + FILE_CREATE_ONLY = O_CREAT|O_EXCL, ///< Only create the file, failing + ///< if it already exists. + FILE_TRUNCATE = O_TRUNC, ///< Truncate the file if it exists. +} FileOpenFlags; + +/// Check whether end of file was encountered +/// +/// @param[in] fp File to check. +/// +/// @return true if it was, false if it was not or read operation was never +/// performed. +static inline bool file_eof(const FileDescriptor *const fp) + FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL + FUNC_ATTR_ALWAYS_INLINE +{ + return fp->eof && rbuffer_size(fp->rv) == 0; +} + +/// Return the file descriptor associated with the FileDescriptor structure +/// +/// @param[in] fp File to check. +/// +/// @return File descriptor. +static inline int file_fd(const FileDescriptor *const fp) + FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL + FUNC_ATTR_ALWAYS_INLINE +{ + return fp->fd; +} + +#ifdef INCLUDE_GENERATED_DECLARATIONS +# include "file.h.generated.h" +#endif +#endif // NVIM_FILE_H diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c index 08122828bd..1fd3987b97 100644 --- a/src/nvim/os/fs.c +++ b/src/nvim/os/fs.c @@ -1,14 +1,20 @@ // fs.c -- filesystem access #include - +#include #include +#include +#include #include +#include + +#include #include "nvim/os/os.h" #include "nvim/os/os_defs.h" #include "nvim/ascii.h" #include "nvim/memory.h" #include "nvim/message.h" +#include "nvim/assert.h" #include "nvim/misc1.h" #include "nvim/misc2.h" #include "nvim/path.h" @@ -18,6 +24,20 @@ # include "os/fs.c.generated.h" #endif +#define RUN_UV_FS_FUNC(ret, func, ...) \ + do { \ + bool did_try_to_free = false; \ +uv_call_start: {} \ + uv_fs_t req; \ + ret = func(&fs_loop, &req, __VA_ARGS__); \ + uv_fs_req_cleanup(&req); \ + if (ret == UV_ENOMEM && !did_try_to_free) { \ + try_to_free_memory(); \ + did_try_to_free = true; \ + goto uv_call_start; \ + } \ + } while (0) + // Many fs functions from libuv return that value on success. static const int kLibuvSuccess = 0; static uv_loop_t fs_loop; @@ -325,13 +345,123 @@ static bool is_executable_in_path(const char_u *name, char_u **abspath) int os_open(const char* path, int flags, int mode) FUNC_ATTR_NONNULL_ALL { - uv_fs_t open_req; - int r = uv_fs_open(&fs_loop, &open_req, path, flags, mode, NULL); - uv_fs_req_cleanup(&open_req); - // r is the same as open_req.result (except for OOM: then only r is set). + int r; + RUN_UV_FS_FUNC(r, uv_fs_open, path, flags, mode, NULL); return r; } +/// Close a file +/// +/// @return 0 or libuv error code on failure. +int os_close(const int fd) +{ + int r; + RUN_UV_FS_FUNC(r, uv_fs_close, fd, NULL); + return r; +} + +/// Read from a file +/// +/// Handles EINTR and ENOMEM, but not other errors. +/// +/// @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] ret_buf Buffer to write to. May be NULL if size is zero. +/// @param[in] size Amount of bytes to read. +/// +/// @return Number of bytes read or libuv error code (< 0). +ptrdiff_t os_read(const int fd, bool *ret_eof, char *const ret_buf, + const size_t size) + FUNC_ATTR_WARN_UNUSED_RESULT +{ + *ret_eof = false; + if (ret_buf == NULL) { + assert(size == 0); + return 0; + } + size_t read_bytes = 0; + bool did_try_to_free = false; + while (read_bytes != size) { + const ptrdiff_t cur_read_bytes = read(fd, ret_buf + read_bytes, + size - read_bytes); + if (cur_read_bytes > 0) { + read_bytes += (size_t) cur_read_bytes; + assert(read_bytes <= size); + } + 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; + } + } + if (cur_read_bytes == 0) { + *ret_eof = true; + break; + } + } + return (ptrdiff_t) read_bytes; +} + +/// Write to a file +/// +/// @param[in] fd File descriptor to write to. +/// @param[in] buf Data to write. May be NULL if size is zero. +/// @param[in] size Amount of bytes to write. +/// +/// @return Number of bytes written or libuv error code (< 0). +ptrdiff_t os_write(const int fd, const char *const buf, const size_t size) + FUNC_ATTR_WARN_UNUSED_RESULT +{ + if (buf == NULL) { + assert(size == 0); + return 0; + } + size_t written_bytes = 0; + while (written_bytes != size) { + const ptrdiff_t cur_written_bytes = write(fd, buf + written_bytes, + size - written_bytes); + if (cur_written_bytes > 0) { + written_bytes += (size_t) cur_written_bytes; + } + if (cur_written_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"); + // According to the man page open() may fail with ENOMEM, but write() + // can’t. +#endif + errno = 0; + if (error == UV_EINTR || error == UV_EAGAIN) { + continue; + } else { + return error; + } + } + if (cur_written_bytes == 0) { + return UV_UNKNOWN; + } + } + return (ptrdiff_t) written_bytes; +} + /// Flushes file modifications to disk. /// /// @param fd the file descriptor of the file to flush to disk. @@ -339,9 +469,8 @@ int os_open(const char* path, int flags, int mode) /// @return `0` on success, a libuv error code on failure. int os_fsync(int fd) { - uv_fs_t fsync_req; - int r = uv_fs_fsync(&fs_loop, &fsync_req, fd, NULL); - uv_fs_req_cleanup(&fsync_req); + int r; + RUN_UV_FS_FUNC(r, uv_fs_fsync, fd, NULL); return r; } @@ -379,16 +508,9 @@ int32_t os_getperm(const char_u *name) int os_setperm(const char_u *name, int perm) FUNC_ATTR_NONNULL_ALL { - uv_fs_t request; - int result = uv_fs_chmod(&fs_loop, &request, - (const char*)name, perm, NULL); - uv_fs_req_cleanup(&request); - - if (result == kLibuvSuccess) { - return OK; - } - - return FAIL; + int r; + RUN_UV_FS_FUNC(r, uv_fs_chmod, (const char *)name, perm, NULL); + return (r == kLibuvSuccess ? OK : FAIL); } /// Changes the ownership of the file referred to by the open file descriptor. @@ -397,13 +519,11 @@ int os_setperm(const char_u *name, int perm) /// /// @note If the `owner` or `group` is specified as `-1`, then that ID is not /// changed. -int os_fchown(int file_descriptor, uv_uid_t owner, uv_gid_t group) +int os_fchown(int fd, uv_uid_t owner, uv_gid_t group) { - uv_fs_t request; - int result = uv_fs_fchown(&fs_loop, &request, file_descriptor, - owner, group, NULL); - uv_fs_req_cleanup(&request); - return result; + int r; + RUN_UV_FS_FUNC(r, uv_fs_fchown, fd, owner, group, NULL); + return r; } /// Check if a file exists. @@ -422,9 +542,8 @@ bool os_file_exists(const char_u *name) bool os_file_is_readable(const char *name) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - uv_fs_t req; - int r = uv_fs_access(&fs_loop, &req, name, R_OK, NULL); - uv_fs_req_cleanup(&req); + int r; + RUN_UV_FS_FUNC(r, uv_fs_access, name, R_OK, NULL); return (r == 0); } @@ -436,9 +555,8 @@ bool os_file_is_readable(const char *name) int os_file_is_writable(const char *name) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - uv_fs_t req; - int r = uv_fs_access(&fs_loop, &req, name, W_OK, NULL); - uv_fs_req_cleanup(&req); + int r; + RUN_UV_FS_FUNC(r, uv_fs_access, name, W_OK, NULL); if (r == 0) { return os_isdir((char_u *)name) ? 2 : 1; } @@ -451,16 +569,10 @@ int os_file_is_writable(const char *name) int os_rename(const char_u *path, const char_u *new_path) FUNC_ATTR_NONNULL_ALL { - uv_fs_t request; - int result = uv_fs_rename(&fs_loop, &request, - (const char *)path, (const char *)new_path, NULL); - uv_fs_req_cleanup(&request); - - if (result == kLibuvSuccess) { - return OK; - } - - return FAIL; + int r; + RUN_UV_FS_FUNC(r, uv_fs_rename, (const char *)path, (const char *)new_path, + NULL); + return (r == kLibuvSuccess ? OK : FAIL); } /// Make a directory. @@ -469,10 +581,9 @@ int os_rename(const char_u *path, const char_u *new_path) int os_mkdir(const char *path, int32_t mode) FUNC_ATTR_NONNULL_ALL { - uv_fs_t request; - int result = uv_fs_mkdir(&fs_loop, &request, path, mode, NULL); - uv_fs_req_cleanup(&request); - return result; + int r; + RUN_UV_FS_FUNC(r, uv_fs_mkdir, path, mode, NULL); + return r; } /// Make a directory, with higher levels when needed @@ -554,10 +665,9 @@ int os_mkdtemp(const char *template, char *path) int os_rmdir(const char *path) FUNC_ATTR_NONNULL_ALL { - uv_fs_t request; - int result = uv_fs_rmdir(&fs_loop, &request, path, NULL); - uv_fs_req_cleanup(&request); - return result; + int r; + RUN_UV_FS_FUNC(r, uv_fs_rmdir, path, NULL); + return r; } /// Opens a directory. @@ -599,10 +709,9 @@ void os_closedir(Directory *dir) int os_remove(const char *path) FUNC_ATTR_NONNULL_ALL { - uv_fs_t request; - int result = uv_fs_unlink(&fs_loop, &request, path, NULL); - uv_fs_req_cleanup(&request); - return result; + int r; + RUN_UV_FS_FUNC(r, uv_fs_unlink, path, NULL); + return r; } /// Get the file information for a given path diff --git a/src/nvim/shada.c b/src/nvim/shada.c index 380d955f63..a8efaeb7cf 100644 --- a/src/nvim/shada.c +++ b/src/nvim/shada.c @@ -5,7 +5,6 @@ #include #include #include -#include #include #include @@ -36,6 +35,7 @@ #include "nvim/version.h" #include "nvim/path.h" #include "nvim/fileio.h" +#include "nvim/file.h" #include "nvim/strings.h" #include "nvim/quickfix.h" #include "nvim/eval/encode.h" @@ -409,7 +409,7 @@ typedef struct sd_read_def { ShaDaFileSkipper skip; ///< Function used to skip some bytes. void *cookie; ///< Data describing object read from. bool eof; ///< True if reader reached end of file. - char *error; ///< Error message in case of error. + const char *error; ///< Error message in case of error. uintmax_t fpos; ///< Current position (amount of bytes read since ///< reader structure initialization). May overflow. vimconv_T sd_conv; ///< Structure used for converting encodings of some @@ -433,7 +433,7 @@ typedef struct sd_write_def { ShaDaFileWriter write; ///< Writer function. ShaDaWriteCloser close; ///< Close function. void *cookie; ///< Data describing object written to. - char *error; ///< Error message in case of error. + const char *error; ///< Error message in case of error. vimconv_T sd_conv; ///< Structure used for converting encodings of some ///< items. } ShaDaWriteDef; @@ -666,38 +666,14 @@ static ptrdiff_t read_file(ShaDaReadDef *const sd_reader, void *const dest, const size_t size) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - size_t read_bytes = 0; - bool did_try_to_free = false; - const int fd = (int)(intptr_t) sd_reader->cookie; - while (read_bytes != size) { - const ptrdiff_t cur_read_bytes = read(fd, ((char *) dest) + read_bytes, - size - read_bytes); - if (cur_read_bytes > 0) { - read_bytes += (size_t) cur_read_bytes; - sd_reader->fpos += (uintmax_t) cur_read_bytes; - assert(read_bytes <= size); - } - if (cur_read_bytes < 0) { - if (errno == EINTR || errno == EAGAIN) { - errno = 0; - continue; - } else if (errno == ENOMEM && !did_try_to_free) { - try_to_free_memory(); - did_try_to_free = true; - errno = 0; - continue; - } else { - sd_reader->error = strerror(errno); - errno = 0; - return -1; - } - } - if (cur_read_bytes == 0) { - sd_reader->eof = true; - break; - } + const ptrdiff_t ret = file_read(sd_reader->cookie, dest, size); + sd_reader->eof = file_eof(sd_reader->cookie); + if (ret < 0) { + sd_reader->error = os_strerror((int)ret); + return -1; } - return (ptrdiff_t) read_bytes; + sd_reader->fpos += (size_t) ret; + return ret; } /// Read one character @@ -720,50 +696,31 @@ static ptrdiff_t write_file(ShaDaWriteDef *const sd_writer, const size_t size) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - size_t written_bytes = 0; - const int fd = (int)(intptr_t) sd_writer->cookie; - while (written_bytes != size) { - const ptrdiff_t cur_written_bytes = write(fd, (char *) dest + written_bytes, - size - written_bytes); - if (cur_written_bytes > 0) { - written_bytes += (size_t) cur_written_bytes; - } - if (cur_written_bytes < 0) { - if (errno == EINTR || errno == EAGAIN) { - errno = 0; - continue; - } else { - sd_writer->error = strerror(errno); - errno = 0; - return -1; - } - } - if (cur_written_bytes == 0) { - sd_writer->error = "Zero bytes written."; - return -1; - } + const ptrdiff_t ret = file_write(sd_writer->cookie, dest, size); + if (ret < 0) { + sd_writer->error = os_strerror((int)ret); + return -1; } - return (ptrdiff_t) written_bytes; + return ret; } /// Wrapper for closing file descriptors opened for reading static void close_sd_reader(ShaDaReadDef *const sd_reader) FUNC_ATTR_NONNULL_ALL { - close_file((int)(intptr_t) sd_reader->cookie); + close_file(sd_reader->cookie); } /// Wrapper for closing file descriptors opened for writing static void close_sd_writer(ShaDaWriteDef *const sd_writer) FUNC_ATTR_NONNULL_ALL { - const int fd = (int)(intptr_t) sd_writer->cookie; - if (os_fsync(fd) < 0) { + const int error = file_fsync(sd_writer->cookie); + if (error < 0) { emsgf(_(SERR "System error while synchronizing ShaDa file: %s"), - os_strerror(errno)); - errno = 0; + os_strerror(error)); } - close_file(fd); + close_file(sd_writer->cookie); } /// Wrapper for read that reads to IObuff and ignores bytes read @@ -779,19 +736,20 @@ static int sd_reader_skip_read(ShaDaReadDef *const sd_reader, const size_t offset) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - size_t read_bytes = 0; - do { - ptrdiff_t new_read_bytes = sd_reader->read( - sd_reader, IObuff, (size_t) (offset - read_bytes > IOSIZE - ? IOSIZE - : offset - read_bytes)); - if (new_read_bytes == -1) { - return FAIL; + const ptrdiff_t skip_bytes = file_skip(sd_reader->cookie, offset); + if (skip_bytes < 0) { + sd_reader->error = os_strerror((int)skip_bytes); + return FAIL; + } else if (skip_bytes != (ptrdiff_t)offset) { + assert(skip_bytes < (ptrdiff_t)offset); + sd_reader->eof = file_eof(sd_reader->cookie); + if (!sd_reader->eof) { + sd_reader->error = _("too few bytes read"); } - read_bytes += (size_t) new_read_bytes; - } while (read_bytes < offset && !sd_reader->eof); - - return (read_bytes == offset ? OK : FAIL); + return FAIL; + } + sd_reader->fpos += (size_t)skip_bytes; + return OK; } /// Wrapper for read that can be used when lseek cannot be used @@ -824,37 +782,6 @@ static ShaDaReadResult sd_reader_skip(ShaDaReadDef *const sd_reader, return kSDReadStatusSuccess; } -/// Wrapper for opening file descriptors -/// -/// All arguments are passed to os_open(). -/// -/// @return file descriptor or libuv error on failure. -static int open_file(const char *const fname, const int flags, const int mode) - FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL -{ - bool did_try_to_free = false; - int fd; -open_file_start: - fd = os_open(fname, flags, mode); - - if (fd < 0) { - if (fd == UV_ENOENT) { - return fd; - } - if (fd == UV_ENOMEM && !did_try_to_free) { - try_to_free_memory(); - did_try_to_free = true; - goto open_file_start; - } - if (fd != UV_EEXIST) { - emsgf(_(SERR "System error while opening ShaDa file %s: %s"), - fname, os_strerror(fd)); - } - return fd; - } - return fd; -} - /// Open ShaDa file for reading /// /// @param[in] fname File name to open. @@ -865,11 +792,7 @@ static int open_shada_file_for_reading(const char *const fname, ShaDaReadDef *sd_reader) FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL { - const intptr_t fd = (intptr_t) open_file(fname, O_RDONLY, 0); - - if (fd < 0) { - return (int) fd; - } + int error; *sd_reader = (ShaDaReadDef) { .read = &read_file, @@ -878,8 +801,11 @@ static int open_shada_file_for_reading(const char *const fname, .error = NULL, .eof = false, .fpos = 0, - .cookie = (void *) fd, + .cookie = file_open_new(&error, fname, FILE_READ_ONLY, 0), }; + if (sd_reader->cookie == NULL) { + return error; + } convert_setup(&sd_reader->sd_conv, "utf-8", p_enc); @@ -887,18 +813,12 @@ static int open_shada_file_for_reading(const char *const fname, } /// Wrapper for closing file descriptors -static void close_file(int fd) +static void close_file(void *cookie) { -close_file_start: - if (close(fd) == -1) { - if (errno == EINTR) { - errno = 0; - goto close_file_start; - } else { - emsgf(_(SERR "System error while closing ShaDa file: %s"), - strerror(errno)); - errno = 0; - } + const int error = file_free(cookie); + if (error != 0) { + emsgf(_(SERR "System error while closing ShaDa file: %s"), + os_strerror(error)); } } @@ -978,7 +898,7 @@ static int shada_read_file(const char *const file, const int flags) } if (of_ret != 0) { - if (of_ret == UV_ENOENT && (flags & kShaDaMissingError)) { + if (of_ret != UV_ENOENT || (flags & kShaDaMissingError)) { emsgf(_(SERR "System error while opening ShaDa file %s for reading: %s"), fname, os_strerror(of_ret)); } @@ -2975,10 +2895,16 @@ int shada_write_file(const char *const file, bool nomerge) }; ShaDaReadDef sd_reader; - intptr_t fd; - if (!nomerge) { - if (open_shada_file_for_reading(fname, &sd_reader) != 0) { + int error; + if ((error = open_shada_file_for_reading(fname, &sd_reader)) != 0) { + if (error != UV_ENOENT) { + emsgf(_(SERR "System error while opening ShaDa file %s for reading " + "to merge before writing it: %s"), + fname, os_strerror(error)); + // Try writing the file even if opening it emerged any issues besides + // file not existing: maybe writing will succeed nevertheless. + } nomerge = true; goto shada_write_file_nomerge; } @@ -2996,15 +2922,12 @@ int shada_write_file(const char *const file, bool nomerge) // 2: Make sure that user can always read and write the result. // 3: If somebody happened to delete the file after it was opened for // reading use u=rw permissions. -shada_write_file_open: - fd = (intptr_t) open_file(tempname, O_CREAT|O_WRONLY|O_NOFOLLOW|O_EXCL, - perm); - if (fd < 0) { - if (fd == UV_EEXIST -#ifdef ELOOP - || fd == UV_ELOOP -#endif - ) { +shada_write_file_open: {} + sd_writer.cookie = file_open_new( + &error, tempname, FILE_CREATE_ONLY|FILE_NOSYMLINK|FILE_WRITE_ONLY, + perm); + if (sd_writer.cookie == NULL) { + if (error == UV_EEXIST || error == UV_ELOOP) { // File already exists, try another name char *const wp = tempname + strlen(tempname) - 1; if (*wp == 'z') { @@ -3019,6 +2942,9 @@ shada_write_file_open: (*wp)++; goto shada_write_file_open; } + } else { + emsgf(_(SERR "System error while opening temporary ShaDa file %s " + "for writing: %s"), tempname, os_strerror(error)); } } } @@ -3042,23 +2968,26 @@ shada_write_file_nomerge: {} } *tail = tail_save; } - fd = (intptr_t) open_file(fname, O_CREAT|O_WRONLY|O_TRUNC, - 0600); - } - - if (p_verbose > 0) { - verbose_enter(); - smsg(_("Writing ShaDa file \"%s\""), fname); - verbose_leave(); + int error; + sd_writer.cookie = file_open_new( + &error, fname, FILE_CREATE|FILE_WRITE_ONLY|FILE_TRUNCATE, 0600); + if (sd_writer.cookie == NULL) { + emsgf(_(SERR "System error while opening ShaDa file %s for writing: %s"), + fname, os_strerror(error)); + } } - if (fd < 0) { + if (sd_writer.cookie == NULL) { xfree(fname); xfree(tempname); return FAIL; } - sd_writer.cookie = (void *) fd; + if (p_verbose > 0) { + verbose_enter(); + smsg(_("Writing ShaDa file \"%s\""), fname); + verbose_leave(); + } convert_setup(&sd_writer.sd_conv, p_enc, "utf-8"); @@ -3085,7 +3014,8 @@ shada_write_file_nomerge: {} || old_info.stat.st_gid != getgid()) { const uv_uid_t old_uid = (uv_uid_t) old_info.stat.st_uid; const uv_gid_t old_gid = (uv_gid_t) old_info.stat.st_gid; - const int fchown_ret = os_fchown((int) fd, old_uid, old_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"), -- cgit From 516b7071cafd831ea563b73f6953232f5674cd0c Mon Sep 17 00:00:00 2001 From: ZyX Date: Sat, 4 Jun 2016 22:48:29 +0300 Subject: file: Add buffered reading and writing Still no busted tests. Not tested without HAVE_PREADV. --- src/nvim/file.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++-- src/nvim/file.h | 4 ++ src/nvim/os/fs.c | 73 ++++++++++++++++++++++++++++ src/nvim/rbuffer.c | 2 +- src/nvim/shada.c | 40 +++++---------- 5 files changed, 225 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/nvim/file.c b/src/nvim/file.c index bc230ecf00..262343fe29 100644 --- a/src/nvim/file.c +++ b/src/nvim/file.c @@ -5,26 +5,39 @@ /// replacement. #include +#include #include #include +#include "auto/config.h" + +#ifdef HAVE_SYS_UIO_H +# include +#endif + #include #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( diff --git a/src/nvim/file.h b/src/nvim/file.h index 0aa98e0def..c6def673c2 100644 --- a/src/nvim/file.h +++ b/src/nvim/file.h @@ -6,10 +6,14 @@ #include #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; diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c index 1fd3987b97..19d4cbc440 100644 --- a/src/nvim/os/fs.c +++ b/src/nvim/os/fs.c @@ -7,6 +7,12 @@ #include #include +#include "auto/config.h" + +#ifdef HAVE_SYS_UIO_H +# include +#endif + #include #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. diff --git a/src/nvim/rbuffer.c b/src/nvim/rbuffer.c index b3805a3a28..d4cc8c0d5d 100644 --- a/src/nvim/rbuffer.c +++ b/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; diff --git a/src/nvim/shada.c b/src/nvim/shada.c index a8efaeb7cf..130d8bbfae 100644 --- a/src/nvim/shada.c +++ b/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) { - emsgf(_(SERR "System error while reading ShaDa file: %s"), - sd_reader->error); - return kSDReadStatusReadError; - } else if (sd_reader->eof) { - emsgf(_(RCERR "Error while reading ShaDa file: " - "last entry specified that it occupies %" PRIu64 " bytes, " - "but file ended earlier"), - (uint64_t) length); - return kSDReadStatusNotShaDa; + if (read_bytes != (ptrdiff_t)length) { + if (sd_reader->error != NULL) { + emsgf(_(SERR "System error while reading ShaDa file: %s"), + sd_reader->error); + return kSDReadStatusReadError; + } else { + emsgf(_(RCERR "Error while reading ShaDa file: " + "last entry specified that it occupies %" PRIu64 " bytes, " + "but file ended earlier"), + (uint64_t) length); + return kSDReadStatusNotShaDa; + } } - assert(read_bytes >= 0 && (size_t) read_bytes == length); return kSDReadStatusSuccess; } -- cgit From a8f3849bc04cee5d7166138a28cb1e7bbf300803 Mon Sep 17 00:00:00 2001 From: ZyX Date: Tue, 21 Jun 2016 22:18:03 +0300 Subject: file: Use own constants, do not rely on fcntl.h One of the reasons is that O_RDONLY is zero, which makes checking whether file is opened read- or write-only harder. It is not guaranteed that on other system O_WRONLY will not be zero (e.g. because file can only be opened in read-write mode). --- src/nvim/file.c | 27 +++++++++++++++++++++++++-- src/nvim/file.h | 24 ++++++++++++------------ src/nvim/shada.c | 13 ++++++------- 3 files changed, 43 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/nvim/file.c b/src/nvim/file.c index 262343fe29..35c659f7fd 100644 --- a/src/nvim/file.c +++ b/src/nvim/file.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "auto/config.h" @@ -46,15 +47,37 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname, const int flags, const int mode) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { + int os_open_flags = 0; int fd; + TriState wr = kNone; +#define FLAG(flags, flag, fcntl_flags, wrval, cond) \ + do { \ + if (flags & flag) { \ + os_open_flags |= fcntl_flags; \ + assert(cond); \ + if (wrval != kNone) { \ + wr = wrval; \ + } \ + } \ + } while (0) + FLAG(flags, kFileWriteOnly, O_WRONLY, kTrue, true); + FLAG(flags, kFileCreateOnly, O_CREAT|O_EXCL|O_WRONLY, kTrue, true); + FLAG(flags, kFileCreate, O_CREAT|O_WRONLY, kTrue, !(flags & kFileCreateOnly)); + FLAG(flags, kFileTruncate, O_TRUNC|O_WRONLY, kTrue, + !(flags & kFileCreateOnly)); + FLAG(flags, kFileReadOnly, O_RDONLY, kFalse, wr != kTrue); +#ifdef O_NOFOLLOW + FLAG(flags, kFileNoSymlink, O_NOFOLLOW, kNone, true); +#endif +#undef FLAG - fd = os_open(fname, flags, mode); + fd = os_open(fname, os_open_flags, mode); if (fd < 0) { return fd; } - ret_fp->wr = (bool)(!!(flags & FILE_WRITE_ONLY)); + ret_fp->wr = (wr == kTrue); ret_fp->fd = fd; ret_fp->eof = false; ret_fp->rv = rbuffer_new(RWBUFSIZE); diff --git a/src/nvim/file.h b/src/nvim/file.h index c6def673c2..5ee572750d 100644 --- a/src/nvim/file.h +++ b/src/nvim/file.h @@ -3,7 +3,6 @@ #include #include -#include #include "nvim/func_attr.h" #include "nvim/rbuffer.h" @@ -19,17 +18,18 @@ typedef struct { /// file_open() flags typedef enum { - FILE_READ_ONLY = O_RDONLY, ///< Open file read-only. - FILE_CREATE = O_CREAT, ///< Create file if it does not exist yet. - FILE_WRITE_ONLY = O_WRONLY, ///< Open file for writing only. -#ifdef O_NOFOLLOW - FILE_NOSYMLINK = O_NOFOLLOW, ///< Do not allow symbolic links. -#else - FILE_NOSYMLINK = 0, -#endif - FILE_CREATE_ONLY = O_CREAT|O_EXCL, ///< Only create the file, failing - ///< if it already exists. - FILE_TRUNCATE = O_TRUNC, ///< Truncate the file if it exists. + kFileReadOnly = 1, ///< Open file read-only. Default. + kFileCreate = 2, ///< Create file if it does not exist yet. + ///< Implies kFileWriteOnly. + kFileWriteOnly = 4, ///< Open file for writing only. + ///< Cannot be used with kFileReadOnly. + kFileNoSymlink = 8, ///< Do not allow symbolic links. + kFileCreateOnly = 16, ///< Only create the file, failing if it already + ///< exists. Implies kFileWriteOnly. Cannot be used + ///< with kFileCreate. + kFileTruncate = 32, ///< Truncate the file if it exists. + ///< Implies kFileWriteOnly. Cannot be used with + ///< kFileCreateOnly. } FileOpenFlags; /// Check whether end of file was encountered diff --git a/src/nvim/shada.c b/src/nvim/shada.c index 130d8bbfae..b4695603b2 100644 --- a/src/nvim/shada.c +++ b/src/nvim/shada.c @@ -672,7 +672,7 @@ static ptrdiff_t read_file(ShaDaReadDef *const sd_reader, void *const dest, sd_reader->error = os_strerror((int)ret); return -1; } - sd_reader->fpos += (size_t) ret; + sd_reader->fpos += (size_t)ret; return ret; } @@ -796,7 +796,7 @@ static int open_shada_file_for_reading(const char *const fname, .error = NULL, .eof = false, .fpos = 0, - .cookie = file_open_new(&error, fname, FILE_READ_ONLY, 0), + .cookie = file_open_new(&error, fname, kFileReadOnly, 0), }; if (sd_reader->cookie == NULL) { return error; @@ -1336,7 +1336,7 @@ static void shada_read(ShaDaReadDef *const sd_reader, const int flags) } } if (!op_register_set(cur_entry.data.reg.name, (yankreg_T) { - .y_array = (char_u **) cur_entry.data.reg.contents, + .y_array = (char_u **)cur_entry.data.reg.contents, .y_size = cur_entry.data.reg.contents_size, .y_type = cur_entry.data.reg.type, .y_width = (colnr_T) cur_entry.data.reg.width, @@ -2919,8 +2919,7 @@ int shada_write_file(const char *const file, bool nomerge) // reading use u=rw permissions. shada_write_file_open: {} sd_writer.cookie = file_open_new( - &error, tempname, FILE_CREATE_ONLY|FILE_NOSYMLINK|FILE_WRITE_ONLY, - perm); + &error, tempname, kFileCreateOnly|kFileNoSymlink, perm); if (sd_writer.cookie == NULL) { if (error == UV_EEXIST || error == UV_ELOOP) { // File already exists, try another name @@ -2964,8 +2963,8 @@ shada_write_file_nomerge: {} *tail = tail_save; } int error; - sd_writer.cookie = file_open_new( - &error, fname, FILE_CREATE|FILE_WRITE_ONLY|FILE_TRUNCATE, 0600); + sd_writer.cookie = file_open_new(&error, fname, kFileCreate|kFileTruncate, + 0600); if (sd_writer.cookie == NULL) { emsgf(_(SERR "System error while opening ShaDa file %s for writing: %s"), fname, os_strerror(error)); -- cgit From 4b9d2caec21f2150e4b253d5201809d77a1e0912 Mon Sep 17 00:00:00 2001 From: ZyX Date: Tue, 21 Jun 2016 22:40:09 +0300 Subject: shada: Do not forget to close ShaDa reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously there was file descriptor leak, not detected by sanitizers. Now it is file descriptor leak with a small memory leak which is detected by ASAN what fails one of the tests (actually, “ShaDa support code leaves .tmp.z in-place when there is error in original ShaDa and it has .tmp.a … .tmp.x”, but error is reported at the next test because leaks are not detected until Neovim exit and Neovim exit happens when clear()/reset() is called which happens in before_each only). --- src/nvim/file.c | 2 +- src/nvim/shada.c | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/nvim/file.c b/src/nvim/file.c index 35c659f7fd..2fb0e1cd87 100644 --- a/src/nvim/file.c +++ b/src/nvim/file.c @@ -310,7 +310,7 @@ ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size) break; } read_bytes += (size_t)new_read_bytes; - } while (read_bytes < size && !fp->eof); + } while (read_bytes < size && !file_eof(fp)); return (ptrdiff_t)read_bytes; } diff --git a/src/nvim/shada.c b/src/nvim/shada.c index b4695603b2..a833b958b9 100644 --- a/src/nvim/shada.c +++ b/src/nvim/shada.c @@ -2883,12 +2883,12 @@ int shada_write_file(const char *const file, bool nomerge) char *const fname = shada_filename(file); char *tempname = NULL; - ShaDaWriteDef sd_writer = (ShaDaWriteDef) { + ShaDaWriteDef sd_writer = { .write = &write_file, .close = &close_sd_writer, .error = NULL, }; - ShaDaReadDef sd_reader; + ShaDaReadDef sd_reader = { .close = NULL }; if (!nomerge) { int error; @@ -2931,6 +2931,8 @@ shada_write_file_open: {} fname); xfree(fname); xfree(tempname); + assert(sd_reader.close != NULL); + sd_reader.close(&sd_reader); return FAIL; } else { (*wp)++; @@ -2974,6 +2976,9 @@ shada_write_file_nomerge: {} if (sd_writer.cookie == NULL) { xfree(fname); xfree(tempname); + if (sd_reader.close != NULL) { + sd_reader.close(&sd_reader); + } return FAIL; } -- cgit From fec7983ecd7ac58f4f8ad9f8b4dc8f6937b87099 Mon Sep 17 00:00:00 2001 From: ZyX Date: Wed, 22 Jun 2016 01:01:21 +0300 Subject: unittests: Add tests for file.c Also fixes some errors found. --- src/nvim/file.c | 36 +++++++++++++++++++----------------- src/nvim/file.h | 17 +++++++++++++---- 2 files changed, 32 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/nvim/file.c b/src/nvim/file.c index 2fb0e1cd87..2730e5b5f9 100644 --- a/src/nvim/file.c +++ b/src/nvim/file.c @@ -29,8 +29,6 @@ # include "file.c.generated.h" #endif -#define RWBUFSIZE (IOSIZE - 1) - /// Open file /// /// @param[out] ret_fp Address where information needed for reading from or @@ -80,7 +78,7 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname, ret_fp->wr = (wr == kTrue); ret_fp->fd = fd; ret_fp->eof = false; - ret_fp->rv = rbuffer_new(RWBUFSIZE); + ret_fp->rv = rbuffer_new(kRWBufferSize); ret_fp->_error = 0; if (ret_fp->wr) { ret_fp->rv->data = ret_fp; @@ -162,7 +160,7 @@ int file_fsync(FileDescriptor *const fp) /// Buffer used for writing /// /// Like IObuff, but allows file_\* callers not to care about spoiling it. -static char writebuf[RWBUFSIZE]; +static char writebuf[kRWBufferSize]; /// Function run when RBuffer is full when writing to a file /// @@ -175,7 +173,10 @@ static void file_rb_write_full_cb(RBuffer *const rv, FileDescriptor *const fp) { assert(fp->wr); assert(rv->data == (void *)fp); - const size_t read_bytes = rbuffer_read(rv, writebuf, RWBUFSIZE); + if (rbuffer_size(rv) == 0) { + return; + } + const size_t read_bytes = rbuffer_read(rv, writebuf, kRWBufferSize); const ptrdiff_t wres = os_write(fp->fd, writebuf, read_bytes); if (wres != (ptrdiff_t)read_bytes) { if (wres >= 0) { @@ -215,22 +216,25 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, if (read_remaining) { assert(rbuffer_size(rv) == 0); rbuffer_reset(rv); - if (read_remaining >= RWBUFSIZE) { + if (read_remaining >= kRWBufferSize) { #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; + size_t write_count; struct iovec iov[] = { { .iov_base = buf, .iov_len = read_remaining }, - { .iov_base = rbuffer_read_ptr(rv, &read_count), .iov_len = RWBUFSIZE - }, + { .iov_base = rbuffer_write_ptr(rv, &write_count), + .iov_len = kRWBufferSize }, }; - assert(read_count == RWBUFSIZE); + assert(write_count == kRWBufferSize); 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) { + read_remaining = 0; rbuffer_produced(rv, (size_t)(r_ret - (ptrdiff_t)read_remaining)); + } else { + read_remaining -= (size_t)r_ret; } } else if (r_ret < 0) { return r_ret; @@ -250,8 +254,8 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, 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); + kRWBufferSize); + assert(write_count == kRWBufferSize); if (r_ret > 0) { rbuffer_produced(rv, (size_t)r_ret); } else if (r_ret < 0) { @@ -288,7 +292,7 @@ ptrdiff_t file_write(FileDescriptor *const fp, const char *const buf, /// Buffer used for skipping. Its contents is undefined and should never be /// used. -static char skipbuf[RWBUFSIZE]; +static char skipbuf[kRWBufferSize]; /// Skip some bytes /// @@ -300,10 +304,8 @@ ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size) assert(!fp->wr); size_t read_bytes = 0; do { - ptrdiff_t new_read_bytes = file_read( - fp, skipbuf, (size_t)(size - read_bytes > sizeof(skipbuf) - ? sizeof(skipbuf) - : size - read_bytes)); + const ptrdiff_t new_read_bytes = file_read( + fp, skipbuf, MIN(size - read_bytes, sizeof(skipbuf))); if (new_read_bytes < 0) { return new_read_bytes; } else if (new_read_bytes == 0) { diff --git a/src/nvim/file.h b/src/nvim/file.h index 5ee572750d..2ba415d2b9 100644 --- a/src/nvim/file.h +++ b/src/nvim/file.h @@ -32,6 +32,9 @@ typedef enum { ///< kFileCreateOnly. } FileOpenFlags; +static inline bool file_eof(const FileDescriptor *const fp) + REAL_FATTR_PURE REAL_FATTR_WARN_UNUSED_RESULT REAL_FATTR_NONNULL_ALL; + /// Check whether end of file was encountered /// /// @param[in] fp File to check. @@ -39,24 +42,30 @@ typedef enum { /// @return true if it was, false if it was not or read operation was never /// performed. static inline bool file_eof(const FileDescriptor *const fp) - FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL - FUNC_ATTR_ALWAYS_INLINE { return fp->eof && rbuffer_size(fp->rv) == 0; } +static inline int file_fd(const FileDescriptor *const fp) + REAL_FATTR_PURE REAL_FATTR_WARN_UNUSED_RESULT REAL_FATTR_NONNULL_ALL; + /// Return the file descriptor associated with the FileDescriptor structure /// /// @param[in] fp File to check. /// /// @return File descriptor. static inline int file_fd(const FileDescriptor *const fp) - FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL - FUNC_ATTR_ALWAYS_INLINE { return fp->fd; } +enum { + /// Read or write buffer size + /// + /// Currently equal to (IOSIZE - 1), but they do not need to be connected. + kRWBufferSize = 1024 +}; + #ifdef INCLUDE_GENERATED_DECLARATIONS # include "file.h.generated.h" #endif -- cgit From 2dd154457ceb4bbc122b9e1ad34bf693ee3dd510 Mon Sep 17 00:00:00 2001 From: ZyX Date: Thu, 23 Jun 2016 21:17:24 +0300 Subject: file: Move src/nvim/file.* to src/nvim/os/fileio.* --- src/nvim/file.c | 318 --------------------------------------------------- src/nvim/file.h | 72 ------------ src/nvim/os/fileio.c | 318 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/nvim/os/fileio.h | 72 ++++++++++++ src/nvim/shada.c | 2 +- 5 files changed, 391 insertions(+), 391 deletions(-) delete mode 100644 src/nvim/file.c delete mode 100644 src/nvim/file.h create mode 100644 src/nvim/os/fileio.c create mode 100644 src/nvim/os/fileio.h (limited to 'src') diff --git a/src/nvim/file.c b/src/nvim/file.c deleted file mode 100644 index 2730e5b5f9..0000000000 --- a/src/nvim/file.c +++ /dev/null @@ -1,318 +0,0 @@ -/// @file file.c -/// -/// Buffered reading/writing to a file. Unlike fileio.c this is not dealing with -/// Neovim stuctures for buffer, with autocommands, etc: just fopen/fread/fwrite -/// replacement. - -#include -#include -#include -#include -#include - -#include "auto/config.h" - -#ifdef HAVE_SYS_UIO_H -# include -#endif - -#include - -#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 - -/// 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. 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\*). -/// -/// @return Error code (@see os_strerror()) or 0. -int file_open(FileDescriptor *const ret_fp, const char *const fname, - const int flags, const int mode) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT -{ - int os_open_flags = 0; - int fd; - TriState wr = kNone; -#define FLAG(flags, flag, fcntl_flags, wrval, cond) \ - do { \ - if (flags & flag) { \ - os_open_flags |= fcntl_flags; \ - assert(cond); \ - if (wrval != kNone) { \ - wr = wrval; \ - } \ - } \ - } while (0) - FLAG(flags, kFileWriteOnly, O_WRONLY, kTrue, true); - FLAG(flags, kFileCreateOnly, O_CREAT|O_EXCL|O_WRONLY, kTrue, true); - FLAG(flags, kFileCreate, O_CREAT|O_WRONLY, kTrue, !(flags & kFileCreateOnly)); - FLAG(flags, kFileTruncate, O_TRUNC|O_WRONLY, kTrue, - !(flags & kFileCreateOnly)); - FLAG(flags, kFileReadOnly, O_RDONLY, kFalse, wr != kTrue); -#ifdef O_NOFOLLOW - FLAG(flags, kFileNoSymlink, O_NOFOLLOW, kNone, true); -#endif -#undef FLAG - - fd = os_open(fname, os_open_flags, mode); - - if (fd < 0) { - return fd; - } - - ret_fp->wr = (wr == kTrue); - ret_fp->fd = fd; - ret_fp->eof = false; - ret_fp->rv = rbuffer_new(kRWBufferSize); - 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; -} - -/// Like file_open(), but allocate and return ret_fp -/// -/// @param[out] error Error code, @see os_strerror(). Is set to zero on -/// success. -/// @param[in] fname File name to open. -/// @param[in] flags Flags, @see FileOpenFlags. -/// @param[in] mode Permissions for the newly created file (ignored if flags -/// does not have FILE_CREATE\*). -/// -/// @return [allocated] Opened file or NULL in case of error. -FileDescriptor *file_open_new(int *const error, const char *const fname, - const int flags, const int mode) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT -{ - FileDescriptor *const fp = xmalloc(sizeof(*fp)); - if ((*error = file_open(fp, fname, flags, mode)) != 0) { - xfree(fp); - return NULL; - } - return fp; -} - -/// Close file and free its buffer -/// -/// @param[in,out] fp File to close. -/// -/// @return 0 or error code. -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; - } - return error; -} - -/// Close and free file obtained using file_open_new() -/// -/// @param[in,out] fp File to close. -/// -/// @return 0 or error code. -int file_free(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL -{ - const int ret = file_close(fp); - xfree(fp); - return ret; -} - -/// Flush file modifications to disk -/// -/// @param[in,out] fp File to work with. -/// -/// @return 0 or error code. -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[kRWBufferSize]; - -/// 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); - if (rbuffer_size(rv) == 0) { - return; - } - const size_t read_bytes = rbuffer_read(rv, writebuf, kRWBufferSize); - 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. -/// @param[out] ret_buf Buffer to read to. Must not be NULL. -/// @param[in] size Number of bytes to read. Buffer must have at least ret_buf -/// bytes. -/// -/// @return error_code (< 0) or number of bytes read. -ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, - const size_t size) - FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT -{ - 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 >= kRWBufferSize) { -#ifdef HAVE_READV - // If there is readv() syscall, then take an opportunity to populate - // both target buffer and RBuffer at once, … - size_t write_count; - struct iovec iov[] = { - { .iov_base = buf, .iov_len = read_remaining }, - { .iov_base = rbuffer_write_ptr(rv, &write_count), - .iov_len = kRWBufferSize }, - }; - assert(write_count == kRWBufferSize); - 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) { - read_remaining = 0; - rbuffer_produced(rv, (size_t)(r_ret - (ptrdiff_t)read_remaining)); - } else { - read_remaining -= (size_t)r_ret; - } - } 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), - kRWBufferSize); - assert(write_count == kRWBufferSize); - 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 -/// -/// @param[in] fd File descriptor to write to. -/// @param[in] buf Data to write. May be NULL if size is zero. -/// @param[in] size Amount of bytes to write. -/// -/// @return Number of bytes written or libuv error code (< 0). -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) -{ - 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[kRWBufferSize]; - -/// Skip some bytes -/// -/// This is like `fseek(fp, size, SEEK_CUR)`, but actual implementation simply -/// reads to a buffer and discards the result. -ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size) - FUNC_ATTR_NONNULL_ALL -{ - assert(!fp->wr); - size_t read_bytes = 0; - do { - const ptrdiff_t new_read_bytes = file_read( - fp, skipbuf, MIN(size - read_bytes, sizeof(skipbuf))); - if (new_read_bytes < 0) { - return new_read_bytes; - } else if (new_read_bytes == 0) { - break; - } - read_bytes += (size_t)new_read_bytes; - } while (read_bytes < size && !file_eof(fp)); - - return (ptrdiff_t)read_bytes; -} diff --git a/src/nvim/file.h b/src/nvim/file.h deleted file mode 100644 index 2ba415d2b9..0000000000 --- a/src/nvim/file.h +++ /dev/null @@ -1,72 +0,0 @@ -#ifndef NVIM_FILE_H -#define NVIM_FILE_H - -#include -#include - -#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; - -/// file_open() flags -typedef enum { - kFileReadOnly = 1, ///< Open file read-only. Default. - kFileCreate = 2, ///< Create file if it does not exist yet. - ///< Implies kFileWriteOnly. - kFileWriteOnly = 4, ///< Open file for writing only. - ///< Cannot be used with kFileReadOnly. - kFileNoSymlink = 8, ///< Do not allow symbolic links. - kFileCreateOnly = 16, ///< Only create the file, failing if it already - ///< exists. Implies kFileWriteOnly. Cannot be used - ///< with kFileCreate. - kFileTruncate = 32, ///< Truncate the file if it exists. - ///< Implies kFileWriteOnly. Cannot be used with - ///< kFileCreateOnly. -} FileOpenFlags; - -static inline bool file_eof(const FileDescriptor *const fp) - REAL_FATTR_PURE REAL_FATTR_WARN_UNUSED_RESULT REAL_FATTR_NONNULL_ALL; - -/// Check whether end of file was encountered -/// -/// @param[in] fp File to check. -/// -/// @return true if it was, false if it was not or read operation was never -/// performed. -static inline bool file_eof(const FileDescriptor *const fp) -{ - return fp->eof && rbuffer_size(fp->rv) == 0; -} - -static inline int file_fd(const FileDescriptor *const fp) - REAL_FATTR_PURE REAL_FATTR_WARN_UNUSED_RESULT REAL_FATTR_NONNULL_ALL; - -/// Return the file descriptor associated with the FileDescriptor structure -/// -/// @param[in] fp File to check. -/// -/// @return File descriptor. -static inline int file_fd(const FileDescriptor *const fp) -{ - return fp->fd; -} - -enum { - /// Read or write buffer size - /// - /// Currently equal to (IOSIZE - 1), but they do not need to be connected. - kRWBufferSize = 1024 -}; - -#ifdef INCLUDE_GENERATED_DECLARATIONS -# include "file.h.generated.h" -#endif -#endif // NVIM_FILE_H diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c new file mode 100644 index 0000000000..6abcceb485 --- /dev/null +++ b/src/nvim/os/fileio.c @@ -0,0 +1,318 @@ +/// @file fileio.c +/// +/// Buffered reading/writing to a file. Unlike fileio.c this is not dealing with +/// Neovim stuctures for buffer, with autocommands, etc: just fopen/fread/fwrite +/// replacement. + +#include +#include +#include +#include +#include + +#include "auto/config.h" + +#ifdef HAVE_SYS_UIO_H +# include +#endif + +#include + +#include "nvim/os/fileio.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 "os/fileio.c.generated.h" +#endif + +/// 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. 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\*). +/// +/// @return Error code (@see os_strerror()) or 0. +int file_open(FileDescriptor *const ret_fp, const char *const fname, + const int flags, const int mode) + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT +{ + int os_open_flags = 0; + int fd; + TriState wr = kNone; +#define FLAG(flags, flag, fcntl_flags, wrval, cond) \ + do { \ + if (flags & flag) { \ + os_open_flags |= fcntl_flags; \ + assert(cond); \ + if (wrval != kNone) { \ + wr = wrval; \ + } \ + } \ + } while (0) + FLAG(flags, kFileWriteOnly, O_WRONLY, kTrue, true); + FLAG(flags, kFileCreateOnly, O_CREAT|O_EXCL|O_WRONLY, kTrue, true); + FLAG(flags, kFileCreate, O_CREAT|O_WRONLY, kTrue, !(flags & kFileCreateOnly)); + FLAG(flags, kFileTruncate, O_TRUNC|O_WRONLY, kTrue, + !(flags & kFileCreateOnly)); + FLAG(flags, kFileReadOnly, O_RDONLY, kFalse, wr != kTrue); +#ifdef O_NOFOLLOW + FLAG(flags, kFileNoSymlink, O_NOFOLLOW, kNone, true); +#endif +#undef FLAG + + fd = os_open(fname, os_open_flags, mode); + + if (fd < 0) { + return fd; + } + + ret_fp->wr = (wr == kTrue); + ret_fp->fd = fd; + ret_fp->eof = false; + ret_fp->rv = rbuffer_new(kRWBufferSize); + 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; +} + +/// Like file_open(), but allocate and return ret_fp +/// +/// @param[out] error Error code, @see os_strerror(). Is set to zero on +/// success. +/// @param[in] fname File name to open. +/// @param[in] flags Flags, @see FileOpenFlags. +/// @param[in] mode Permissions for the newly created file (ignored if flags +/// does not have FILE_CREATE\*). +/// +/// @return [allocated] Opened file or NULL in case of error. +FileDescriptor *file_open_new(int *const error, const char *const fname, + const int flags, const int mode) + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_MALLOC FUNC_ATTR_WARN_UNUSED_RESULT +{ + FileDescriptor *const fp = xmalloc(sizeof(*fp)); + if ((*error = file_open(fp, fname, flags, mode)) != 0) { + xfree(fp); + return NULL; + } + return fp; +} + +/// Close file and free its buffer +/// +/// @param[in,out] fp File to close. +/// +/// @return 0 or error code. +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; + } + return error; +} + +/// Close and free file obtained using file_open_new() +/// +/// @param[in,out] fp File to close. +/// +/// @return 0 or error code. +int file_free(FileDescriptor *const fp) FUNC_ATTR_NONNULL_ALL +{ + const int ret = file_close(fp); + xfree(fp); + return ret; +} + +/// Flush file modifications to disk +/// +/// @param[in,out] fp File to work with. +/// +/// @return 0 or error code. +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[kRWBufferSize]; + +/// 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); + if (rbuffer_size(rv) == 0) { + return; + } + const size_t read_bytes = rbuffer_read(rv, writebuf, kRWBufferSize); + 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. +/// @param[out] ret_buf Buffer to read to. Must not be NULL. +/// @param[in] size Number of bytes to read. Buffer must have at least ret_buf +/// bytes. +/// +/// @return error_code (< 0) or number of bytes read. +ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, + const size_t size) + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT +{ + 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 >= kRWBufferSize) { +#ifdef HAVE_READV + // If there is readv() syscall, then take an opportunity to populate + // both target buffer and RBuffer at once, … + size_t write_count; + struct iovec iov[] = { + { .iov_base = buf, .iov_len = read_remaining }, + { .iov_base = rbuffer_write_ptr(rv, &write_count), + .iov_len = kRWBufferSize }, + }; + assert(write_count == kRWBufferSize); + 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) { + read_remaining = 0; + rbuffer_produced(rv, (size_t)(r_ret - (ptrdiff_t)read_remaining)); + } else { + read_remaining -= (size_t)r_ret; + } + } 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), + kRWBufferSize); + assert(write_count == kRWBufferSize); + 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 +/// +/// @param[in] fd File descriptor to write to. +/// @param[in] buf Data to write. May be NULL if size is zero. +/// @param[in] size Amount of bytes to write. +/// +/// @return Number of bytes written or libuv error code (< 0). +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) +{ + 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[kRWBufferSize]; + +/// Skip some bytes +/// +/// This is like `fseek(fp, size, SEEK_CUR)`, but actual implementation simply +/// reads to a buffer and discards the result. +ptrdiff_t file_skip(FileDescriptor *const fp, const size_t size) + FUNC_ATTR_NONNULL_ALL +{ + assert(!fp->wr); + size_t read_bytes = 0; + do { + const ptrdiff_t new_read_bytes = file_read( + fp, skipbuf, MIN(size - read_bytes, sizeof(skipbuf))); + if (new_read_bytes < 0) { + return new_read_bytes; + } else if (new_read_bytes == 0) { + break; + } + read_bytes += (size_t)new_read_bytes; + } while (read_bytes < size && !file_eof(fp)); + + return (ptrdiff_t)read_bytes; +} diff --git a/src/nvim/os/fileio.h b/src/nvim/os/fileio.h new file mode 100644 index 0000000000..2cffd5c851 --- /dev/null +++ b/src/nvim/os/fileio.h @@ -0,0 +1,72 @@ +#ifndef NVIM_OS_FILEIO_H +#define NVIM_OS_FILEIO_H + +#include +#include + +#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; + +/// file_open() flags +typedef enum { + kFileReadOnly = 1, ///< Open file read-only. Default. + kFileCreate = 2, ///< Create file if it does not exist yet. + ///< Implies kFileWriteOnly. + kFileWriteOnly = 4, ///< Open file for writing only. + ///< Cannot be used with kFileReadOnly. + kFileNoSymlink = 8, ///< Do not allow symbolic links. + kFileCreateOnly = 16, ///< Only create the file, failing if it already + ///< exists. Implies kFileWriteOnly. Cannot be used + ///< with kFileCreate. + kFileTruncate = 32, ///< Truncate the file if it exists. + ///< Implies kFileWriteOnly. Cannot be used with + ///< kFileCreateOnly. +} FileOpenFlags; + +static inline bool file_eof(const FileDescriptor *const fp) + REAL_FATTR_PURE REAL_FATTR_WARN_UNUSED_RESULT REAL_FATTR_NONNULL_ALL; + +/// Check whether end of file was encountered +/// +/// @param[in] fp File to check. +/// +/// @return true if it was, false if it was not or read operation was never +/// performed. +static inline bool file_eof(const FileDescriptor *const fp) +{ + return fp->eof && rbuffer_size(fp->rv) == 0; +} + +static inline int file_fd(const FileDescriptor *const fp) + REAL_FATTR_PURE REAL_FATTR_WARN_UNUSED_RESULT REAL_FATTR_NONNULL_ALL; + +/// Return the file descriptor associated with the FileDescriptor structure +/// +/// @param[in] fp File to check. +/// +/// @return File descriptor. +static inline int file_fd(const FileDescriptor *const fp) +{ + return fp->fd; +} + +enum { + /// Read or write buffer size + /// + /// Currently equal to (IOSIZE - 1), but they do not need to be connected. + kRWBufferSize = 1024 +}; + +#ifdef INCLUDE_GENERATED_DECLARATIONS +# include "os/fileio.h.generated.h" +#endif +#endif // NVIM_OS_FILEIO_H diff --git a/src/nvim/shada.c b/src/nvim/shada.c index a833b958b9..5047ef9647 100644 --- a/src/nvim/shada.c +++ b/src/nvim/shada.c @@ -35,7 +35,7 @@ #include "nvim/version.h" #include "nvim/path.h" #include "nvim/fileio.h" -#include "nvim/file.h" +#include "nvim/os/fileio.h" #include "nvim/strings.h" #include "nvim/quickfix.h" #include "nvim/eval/encode.h" -- cgit From 75bd39d49c09896a885ac700862f9b3bb84f6069 Mon Sep 17 00:00:00 2001 From: ZyX Date: Thu, 23 Jun 2016 23:49:23 +0300 Subject: *: Satisfy linter (newest type casts rule) --- src/nvim/os/fs.c | 14 +++++++------- src/nvim/shada.c | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c index 19d4cbc440..49c42cb63d 100644 --- a/src/nvim/os/fs.c +++ b/src/nvim/os/fs.c @@ -392,7 +392,7 @@ ptrdiff_t os_read(const int fd, bool *ret_eof, char *const ret_buf, const ptrdiff_t cur_read_bytes = read(fd, ret_buf + read_bytes, size - read_bytes); if (cur_read_bytes > 0) { - read_bytes += (size_t) cur_read_bytes; + read_bytes += (size_t)cur_read_bytes; assert(read_bytes <= size); } if (cur_read_bytes < 0) { @@ -412,7 +412,7 @@ ptrdiff_t os_read(const int fd, bool *ret_eof, char *const ret_buf, did_try_to_free = true; continue; } else { - return (ptrdiff_t) error; + return (ptrdiff_t)error; } } if (cur_read_bytes == 0) { @@ -420,11 +420,11 @@ ptrdiff_t os_read(const int fd, bool *ret_eof, char *const ret_buf, break; } } - return (ptrdiff_t) read_bytes; + return (ptrdiff_t)read_bytes; } #ifdef HAVE_READV -/// Read from a file to a multiple buffers at once +/// Read from a file to multiple buffers at once /// /// Wrapper for readv(). /// @@ -455,7 +455,7 @@ ptrdiff_t os_readv(int fd, bool *ret_eof, struct iovec *iov, size_t iov_size) 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) { + 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; @@ -509,7 +509,7 @@ ptrdiff_t os_write(const int fd, const char *const buf, const size_t size) const ptrdiff_t cur_written_bytes = write(fd, buf + written_bytes, size - written_bytes); if (cur_written_bytes > 0) { - written_bytes += (size_t) cur_written_bytes; + written_bytes += (size_t)cur_written_bytes; } if (cur_written_bytes < 0) { #ifdef HAVE_UV_TRANSLATE_SYS_ERROR @@ -532,7 +532,7 @@ ptrdiff_t os_write(const int fd, const char *const buf, const size_t size) return UV_UNKNOWN; } } - return (ptrdiff_t) written_bytes; + return (ptrdiff_t)written_bytes; } /// Flushes file modifications to disk. diff --git a/src/nvim/shada.c b/src/nvim/shada.c index 5047ef9647..b5921eb810 100644 --- a/src/nvim/shada.c +++ b/src/nvim/shada.c @@ -3007,8 +3007,8 @@ shada_write_file_nomerge: {} if (getuid() == ROOT_UID) { if (old_info.stat.st_uid != ROOT_UID || old_info.stat.st_gid != getgid()) { - const uv_uid_t old_uid = (uv_uid_t) old_info.stat.st_uid; - const uv_gid_t old_gid = (uv_gid_t) old_info.stat.st_gid; + const uv_uid_t old_uid = (uv_uid_t)old_info.stat.st_uid; + 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); if (fchown_ret != 0) { @@ -3192,7 +3192,7 @@ static ShaDaReadResult fread_len(ShaDaReadDef *const sd_reader, emsgf(_(RCERR "Error while reading ShaDa file: " "last entry specified that it occupies %" PRIu64 " bytes, " "but file ended earlier"), - (uint64_t) length); + (uint64_t)length); return kSDReadStatusNotShaDa; } } -- cgit From 96a57e1bc66f294684e9ac2ee8f2b74243982123 Mon Sep 17 00:00:00 2001 From: ZyX Date: Fri, 24 Jun 2016 01:07:09 +0300 Subject: os/fileio: Use readv often --- src/nvim/os/fileio.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index 6abcceb485..6cee102305 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -216,30 +216,31 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, if (read_remaining) { assert(rbuffer_size(rv) == 0); rbuffer_reset(rv); - if (read_remaining >= kRWBufferSize) { #ifdef HAVE_READV - // If there is readv() syscall, then take an opportunity to populate - // both target buffer and RBuffer at once, … - size_t write_count; - struct iovec iov[] = { - { .iov_base = buf, .iov_len = read_remaining }, - { .iov_base = rbuffer_write_ptr(rv, &write_count), - .iov_len = kRWBufferSize }, - }; - assert(write_count == kRWBufferSize); - 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) { - read_remaining = 0; - rbuffer_produced(rv, (size_t)(r_ret - (ptrdiff_t)read_remaining)); - } else { - read_remaining -= (size_t)r_ret; - } - } else if (r_ret < 0) { - return r_ret; + // If there is readv() syscall, then take an opportunity to populate + // both target buffer and RBuffer at once, … + size_t write_count; + struct iovec iov[] = { + { .iov_base = buf, .iov_len = read_remaining }, + { .iov_base = rbuffer_write_ptr(rv, &write_count), + .iov_len = kRWBufferSize }, + }; + assert(write_count == kRWBufferSize); + 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)); + read_remaining = 0; + } else { + buf += (size_t)r_ret; + read_remaining -= (size_t)r_ret; } + } else if (r_ret < 0) { + return r_ret; + } #else + if (read_remaining >= kRWBufferSize) { // …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); @@ -249,7 +250,6 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, } 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, @@ -262,6 +262,7 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, return r_ret; } } +#endif } } return (ptrdiff_t)(size - read_remaining); -- cgit