diff options
author | James McCoy <jamessan@jamessan.com> | 2017-08-08 22:18:50 +0200 |
---|---|---|
committer | Justin M. Keyes <justinkz@gmail.com> | 2017-08-10 00:56:07 +0200 |
commit | ac055d677aa9eff9fca11cecb5ac7f7a4edb0265 (patch) | |
tree | 4b0b0a74ae39fc3cdad55f244fc2142618e1ceae /src | |
parent | 085102fadf1c1371e863c0e9af8240038ce641e2 (diff) | |
download | rneovim-ac055d677aa9eff9fca11cecb5ac7f7a4edb0265.tar.gz rneovim-ac055d677aa9eff9fca11cecb5ac7f7a4edb0265.tar.bz2 rneovim-ac055d677aa9eff9fca11cecb5ac7f7a4edb0265.zip |
os_stat: return ENOENT on NULL filename arg
Closes #4370
Explication:
In the backtrace in #4370, we see that `buf_write()` was called with
non-NULL `fname` and `sfname` arguments, but they've since _become_
NULL.
#7 0x00000000004de09d in buf_write (buf=0x1dee040, fname=0x0, fname@entry=0x1e985b0 "/home/sean/src/github.com/snczl/virta/pkg/meld/segment.go",
sfname=0x0, sfname@entry=0x1ddfa60 "segment.go", start=1, end=72, eap=eap@entry=0x7ffc6b032e60, append=0,
forceit=0, reset_changed=1, filtering=0)
at /home/travis/build/neovim/bot-ci/build/neovim/src/nvim/fileio.c:2576
This is most likely due to the code that restores those values from
`buf`, which happens just before the fatal call to `os_fileinfo`
```c
/*
* The autocommands may have changed the name of the buffer, which may
* be kept in fname, ffname and sfname.
*/
if (buf_ffname)
ffname = buf->b_ffname;
if (buf_sfname)
sfname = buf->b_sfname;
if (buf_fname_f)
fname = buf->b_ffname;
if (buf_fname_s)
fname = buf->b_sfname;
```
It's worth noting that at this point `ffname` is still non-NULL, so
it _could_ be used. However, our current code is purely more strict
than Vim in this area, which has caused us problems before (e.g.,
`getdigits()`). The commentary for `struct file_buffer` clearly
indicate that all of `b_ffname`, `b_sfname`, and `b_fname` may be
NULL:
```c
/*
* b_ffname has the full path of the file (NULL for no name).
* b_sfname is the name as the user typed it (or NULL).
* b_fname is the same as b_sfname, unless ":cd" has been done,
* then it is the same as b_ffname (NULL for no name).
*/
char_u *b_ffname; /* full path file name */
char_u *b_sfname; /* short file name */
char_u *b_fname; /* current file name */
```
Vim directly calls `stat(2)` which, although it is annotated to tell
the compiler that the path argument is non-NULL, does handle a NULL
pointer by returning a `-1` value and setting `errno` to `EFAULT`.
This satisfies Vim's check, since it treats any `-1` return from
`stat(2)` to mean the file doesn't exist (at least in this code
path).
Note that Vim's mch_stat() implementations on win32 and solaris
clearly cannot accept NULL `name`. But the codepaths that call
mch_stat will NULL `name` tend to be unix-only (eg: u_read_undo)!
Diffstat (limited to 'src')
-rw-r--r-- | src/nvim/os/fs.c | 9 |
1 files changed, 5 insertions, 4 deletions
diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c index 9e3025cf89..6ac9d682d7 100644 --- a/src/nvim/os/fs.c +++ b/src/nvim/os/fs.c @@ -605,8 +605,11 @@ int os_fsync(int fd) /// /// @return libuv return code. static int os_stat(const char *name, uv_stat_t *statbuf) - FUNC_ATTR_NONNULL_ALL + FUNC_ATTR_NONNULL_ARG(2) { + if (!name) { + return UV_ENOENT; + } uv_fs_t request; int result = uv_fs_stat(&fs_loop, &request, name, NULL); *statbuf = request.statbuf; @@ -618,7 +621,6 @@ static int os_stat(const char *name, uv_stat_t *statbuf) /// /// @return libuv error code on error. int32_t os_getperm(const char *name) - FUNC_ATTR_NONNULL_ALL { uv_stat_t statbuf; int stat_result = os_stat(name, &statbuf); @@ -657,7 +659,6 @@ int os_fchown(int fd, uv_uid_t owner, uv_gid_t group) /// /// @return `true` if `path` exists bool os_path_exists(const char_u *path) - FUNC_ATTR_NONNULL_ALL { uv_stat_t statbuf; return os_stat((char *)path, &statbuf) == kLibuvSuccess; @@ -847,7 +848,7 @@ int os_remove(const char *path) /// @param[out] file_info Pointer to a FileInfo to put the information in. /// @return `true` on success, `false` for failure. bool os_fileinfo(const char *path, FileInfo *file_info) - FUNC_ATTR_NONNULL_ALL + FUNC_ATTR_NONNULL_ARG(2) { return os_stat(path, &(file_info->stat)) == kLibuvSuccess; } |