From bd798a3267a496c644b339c45189b09e2a952014 Mon Sep 17 00:00:00 2001 From: ZyX Date: Sun, 19 Mar 2017 16:55:37 +0300 Subject: getchar: Use fileio instead of fdopen Problem: as fileio is cached and reads blocks this is going to wait until either EOF or reading enough characters to fill rbuffer. This is not good when reading user input from stdin as script. --- src/nvim/os/fileio.c | 48 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) (limited to 'src/nvim/os/fileio.c') diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index 775f2bd449..3742fd53de 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -45,7 +45,6 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname, 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 { \ @@ -70,13 +69,33 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname, #endif #undef FLAG - fd = os_open(fname, os_open_flags, mode); + const int fd = os_open(fname, os_open_flags, mode); if (fd < 0) { return fd; } + return file_open_fd(ret_fp, fd, (wr == kTrue), mode); +} - ret_fp->wr = (wr == kTrue); +/// Wrap file descriptor with FileDescriptor structure +/// +/// @warning File descriptor wrapped like this must not be accessed by other +/// means. +/// +/// @param[out] ret_fp Address where information needed for reading from or +/// writing to a file is saved +/// @param[in] fd File descriptor to wrap. +/// @param[in] wr True if fd is opened for writing only, false if it is read +/// only. +/// @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. Currently always returns 0. +int file_open_fd(FileDescriptor *const ret_fp, const int fd, + const bool wr, const int mode) + FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT +{ + ret_fp->wr = wr; ret_fp->fd = fd; ret_fp->eof = false; ret_fp->rv = rbuffer_new(kRWBufferSize); @@ -110,6 +129,29 @@ FileDescriptor *file_open_new(int *const error, const char *const fname, return fp; } +/// Like file_open_fd(), but allocate and return ret_fp +/// +/// @param[out] error Error code, @see os_strerror(). Is set to zero on +/// success. +/// @param[in] fd File descriptor to wrap. +/// @param[in] wr True if fd is opened for writing only, false if it is read +/// only. +/// @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_fd_new(int *const error, const int fd, + const bool wr, 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_fd(fp, fd, wr, mode)) != 0) { + xfree(fp); + return NULL; + } + return fp; +} + /// Close file and free its buffer /// /// @param[in,out] fp File to close. -- cgit From e78e75d85d91e9f14964465ea136b3899b774d6e Mon Sep 17 00:00:00 2001 From: ZyX Date: Sun, 19 Mar 2017 17:29:48 +0300 Subject: fileio,main: Do not restart syscall at EAGAIN when reading for -s --- src/nvim/os/fileio.c | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) (limited to 'src/nvim/os/fileio.c') diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index 3742fd53de..70ed49c3aa 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -74,7 +74,7 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname, if (fd < 0) { return fd; } - return file_open_fd(ret_fp, fd, (wr == kTrue), mode); + return file_open_fd(ret_fp, fd, flags, mode); } /// Wrap file descriptor with FileDescriptor structure @@ -85,17 +85,24 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname, /// @param[out] ret_fp Address where information needed for reading from or /// writing to a file is saved /// @param[in] fd File descriptor to wrap. -/// @param[in] wr True if fd is opened for writing only, false if it is read -/// only. +/// @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. Currently always returns 0. int file_open_fd(FileDescriptor *const ret_fp, const int fd, - const bool wr, const int mode) + const int flags, const int mode) FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT { - ret_fp->wr = wr; + ret_fp->wr = !!(flags & (kFileCreate|kFileCreateOnly + |kFileTruncate + |kFileAppend + |kFileWriteOnly)); + ret_fp->non_blocking = !!(flags & kFileNonBlocking); + // Non-blocking writes not supported currently. + assert(!ret_fp->wr || !ret_fp->non_blocking); ret_fp->fd = fd; ret_fp->eof = false; ret_fp->rv = rbuffer_new(kRWBufferSize); @@ -134,18 +141,17 @@ FileDescriptor *file_open_new(int *const error, const char *const fname, /// @param[out] error Error code, @see os_strerror(). Is set to zero on /// success. /// @param[in] fd File descriptor to wrap. -/// @param[in] wr True if fd is opened for writing only, false if it is read -/// only. +/// @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_fd_new(int *const error, const int fd, - const bool wr, const int mode) + 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_fd(fp, fd, wr, mode)) != 0) { + if ((*error = file_open_fd(fp, fd, flags, mode)) != 0) { xfree(fp); return NULL; } @@ -224,7 +230,8 @@ static void file_rb_write_full_cb(RBuffer *const rv, FileDescriptor *const fp) return; } const size_t read_bytes = rbuffer_read(rv, writebuf, kRWBufferSize); - const ptrdiff_t wres = os_write(fp->fd, writebuf, read_bytes); + const ptrdiff_t wres = os_write(fp->fd, writebuf, read_bytes, + fp->non_blocking); if (wres != (ptrdiff_t)read_bytes) { if (wres >= 0) { fp->_error = UV_EIO; @@ -274,7 +281,7 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, }; assert(write_count == kRWBufferSize); const ptrdiff_t r_ret = os_readv(fp->fd, &fp->eof, iov, - ARRAY_SIZE(iov)); + ARRAY_SIZE(iov), fp->non_blocking); if (r_ret > 0) { if (r_ret > (ptrdiff_t)read_remaining) { rbuffer_produced(rv, (size_t)(r_ret - (ptrdiff_t)read_remaining)); @@ -290,7 +297,8 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, 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); + const ptrdiff_t r_ret = os_read(fp->fd, &fp->eof, buf, read_remaining, + fp->non_blocking); if (r_ret >= 0) { read_remaining -= (size_t)r_ret; return (ptrdiff_t)(size - read_remaining); @@ -301,7 +309,7 @@ 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), - kRWBufferSize); + kRWBufferSize, fp->non_blocking); assert(write_count == kRWBufferSize); if (r_ret > 0) { rbuffer_produced(rv, (size_t)r_ret); @@ -311,6 +319,10 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, } #endif } + if (fp->non_blocking) { + // Allow only at most one os_read[v] call. + break; + } } return (ptrdiff_t)(size - read_remaining); } -- cgit From 83b39a4a027ec2b314e60b53c709611c770cc921 Mon Sep 17 00:00:00 2001 From: ZyX Date: Sun, 19 Mar 2017 18:24:20 +0300 Subject: os/fileio: Fix QB failure --- src/nvim/os/fileio.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/nvim/os/fileio.c') diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index 70ed49c3aa..7668a867a0 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -68,6 +68,10 @@ int file_open(FileDescriptor *const ret_fp, const char *const fname, FLAG(flags, kFileNoSymlink, O_NOFOLLOW, kNone, true); #endif #undef FLAG + // wr is used for kFileReadOnly flag, but on + // QB:neovim-qb-slave-ubuntu-12-04-64bit it still errors out with + // `error: variable ‘wr’ set but not used [-Werror=unused-but-set-variable]` + (void)wr; const int fd = os_open(fname, os_open_flags, mode); -- cgit From 387fbcd95cade4b0c037d18f404944676a59db09 Mon Sep 17 00:00:00 2001 From: b-r-o-c-k Date: Sat, 14 Apr 2018 14:21:36 -0500 Subject: win: Fix reading from stdin * Reading from stdin on Windows is fixed in the same way as it was in #8267. * The file_read function was returning without filling the destination buffer when it was called with a non-blocking file descriptor. --- src/nvim/os/fileio.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src/nvim/os/fileio.c') diff --git a/src/nvim/os/fileio.c b/src/nvim/os/fileio.c index 0ef9307a5a..ccf35fd57c 100644 --- a/src/nvim/os/fileio.c +++ b/src/nvim/os/fileio.c @@ -282,6 +282,7 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, char *buf = ret_buf; size_t read_remaining = size; RBuffer *const rv = fp->rv; + bool called_read = false; while (read_remaining) { const size_t rv_size = rbuffer_size(rv); if (rv_size > 0) { @@ -289,7 +290,9 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, buf += rsize; read_remaining -= rsize; } - if (fp->eof) { + if (fp->eof + // Allow only at most one os_read[v] call. + || (called_read && fp->non_blocking)) { break; } if (read_remaining) { @@ -343,10 +346,7 @@ ptrdiff_t file_read(FileDescriptor *const fp, char *const ret_buf, } } #endif - } - if (fp->non_blocking) { - // Allow only at most one os_read[v] call. - break; + called_read = true; } } return (ptrdiff_t)(size - read_remaining); -- cgit