Merge pull request #3166 from justinmk/file_is_readable

os_file_is_readable()

4 files changed, 80 insertions, 35 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index c7c67cfca4..bf9a219e28 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -8562,27 +8562,12 @@ static void f_feedkeys(typval_T *argvars, typval_T *rettv)
   }
 }
 
-/*
- * "filereadable()" function
- */
+/// "filereadable()" function
 static void f_filereadable(typval_T *argvars, typval_T *rettv)
 {
-  int fd;
-  char_u      *p;
-  int n;
-
-#ifndef O_NONBLOCK
-# define O_NONBLOCK 0
-#endif
-  p = get_tv_string(&argvars[0]);
-  if (*p && !os_isdir(p) && (fd = os_open((char *)p,
-                                  O_RDONLY | O_NONBLOCK, 0)) >= 0) {
-    n = TRUE;
-    close(fd);
-  } else
-    n = FALSE;
-
-  rettv->vval.v_number = n;
+  char_u *p = get_tv_string(&argvars[0]);
+  rettv->vval.v_number =
+    (*p && !os_isdir(p) && os_file_is_readable((char*)p));
 }
 
 /*
diff --git a/src/nvim/os/fs.c b/src/nvim/os/fs.c
index 5eeb275701..e4776999e5 100644
--- a/src/nvim/os/fs.c
+++ b/src/nvim/os/fs.c
@@ -279,19 +279,31 @@ bool os_file_is_readonly(const char *name)
   return access(name, W_OK) != 0;
 }
 
+/// Check if a file is readable.
+///
+/// @return true if `name` is readable, otherwise false.
+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);
+  return (r == 0);
+}
+
 /// Check if a file is writable.
 ///
 /// @return `0` if `name` is not writable,
 /// @return `1` if `name` is writable,
 /// @return `2` for a directory which we have rights to write into.
 int os_file_is_writable(const char *name)
-  FUNC_ATTR_NONNULL_ALL
+  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
 {
-  if (access(name, W_OK) == 0) {
-    if (os_isdir((char_u *)name)) {
-      return 2;
-    }
-    return 1;
+  uv_fs_t req;
+  int r = uv_fs_access(&fs_loop, &req, name, W_OK, NULL);
+  uv_fs_req_cleanup(&req);
+  if (r == 0) {
+    return os_isdir((char_u *)name) ? 2 : 1;
   }
   return 0;
 }
diff --git a/src/nvim/os/win_defs.h b/src/nvim/os/win_defs.h
index 19d796bd08..a51898c9e7 100644
--- a/src/nvim/os/win_defs.h
+++ b/src/nvim/os/win_defs.h
@@ -21,4 +21,12 @@
 // - SYS_VIMRC_FILE
 // - SPECIAL_WILDCHAR
 
+// _access(): https://msdn.microsoft.com/en-us/library/1w06ktdy.aspx
+#ifndef R_OK
+# define R_OK 4
+#endif
+#ifndef W_OK
+# define W_OK 2
+#endif
+
 #endif  // NVIM_OS_WIN_DEFS_H
diff --git a/test/unit/os/fs_spec.lua b/test/unit/os/fs_spec.lua
index f7f0cba951..9cca3ca244 100644
--- a/test/unit/os/fs_spec.lua
+++ b/test/unit/os/fs_spec.lua
@@ -32,6 +32,14 @@ local directory = nil
 local absolute_executable = nil
 local executable_name = nil
 
+local function set_bit(number, to_set)
+  return bit.bor(number, to_set)
+end
+
+local function unset_bit(number, to_unset)
+  return bit.band(number, (bit.bnot(to_unset)))
+end
+
 local function assert_file_exists(filepath)
   neq(nil, lfs.attributes(filepath))
 end
@@ -40,11 +48,24 @@ local function assert_file_does_not_exist(filepath)
   eq(nil, lfs.attributes(filepath))
 end
 
+local function os_setperm(filename, perm)
+  return fs.os_setperm((to_cstr(filename)), perm)
+end
+
+local function os_getperm(filename)
+  local perm = fs.os_getperm((to_cstr(filename)))
+  return tonumber(perm)
+end
+
 describe('fs function', function()
+  local orig_test_file_perm
 
   setup(function()
     lfs.mkdir('unit-test-directory');
+
     io.open('unit-test-directory/test.file', 'w').close()
+    orig_test_file_perm = os_getperm('unit-test-directory/test.file')
+
     io.open('unit-test-directory/test_2.file', 'w').close()
     lfs.link('test.file', 'unit-test-directory/test_link.file', true)
     -- Since the tests are executed, they are called by an executable. We use
@@ -188,6 +209,10 @@ describe('fs function', function()
   end)
 
   describe('file permissions', function()
+    before_each(function()
+      os_setperm('unit-test-directory/test.file', orig_test_file_perm)
+    end)
+
     local function os_getperm(filename)
       local perm = fs.os_getperm((to_cstr(filename)))
       return tonumber(perm)
@@ -208,6 +233,10 @@ describe('fs function', function()
       return fs.os_file_is_readonly((to_cstr(filename)))
     end
 
+    local function os_file_is_readable(filename)
+      return fs.os_file_is_readable((to_cstr(filename)))
+    end
+
     local function os_file_is_writable(filename)
       return fs.os_file_is_writable((to_cstr(filename)))
     end
@@ -216,14 +245,6 @@ describe('fs function', function()
       return 0 ~= (bit.band(number, check_bit))
     end
 
-    local function set_bit(number, to_set)
-      return bit.bor(number, to_set)
-    end
-
-    local function unset_bit(number, to_unset)
-      return bit.band(number, (bit.bnot(to_unset)))
-    end
-
     describe('os_getperm', function()
       it('returns -1 when the given file does not exist', function()
         eq(-1, (os_getperm('non-existing-file')))
@@ -322,16 +343,35 @@ describe('fs function', function()
       end)
     end)
 
+    describe('os_file_is_readable', function()
+      it('returns false if the file is not readable', function()
+        local perm = os_getperm('unit-test-directory/test.file')
+        perm = unset_bit(perm, ffi.C.kS_IRUSR)
+        perm = unset_bit(perm, ffi.C.kS_IRGRP)
+        perm = unset_bit(perm, ffi.C.kS_IROTH)
+        eq(OK, (os_setperm('unit-test-directory/test.file', perm)))
+        eq(false, os_file_is_readable('unit-test-directory/test.file'))
+      end)
+
+      it('returns false if the file does not exist', function()
+        eq(false, os_file_is_readable(
+          'unit-test-directory/what_are_you_smoking.gif'))
+      end)
+
+      it('returns true if the file is readable', function()
+        eq(true, os_file_is_readable(
+          'unit-test-directory/test.file'))
+      end)
+    end)
+
     describe('os_file_is_writable', function()
       it('returns 0 if the file is readonly', function()
         local perm = os_getperm('unit-test-directory/test.file')
-        local perm_orig = perm
         perm = unset_bit(perm, ffi.C.kS_IWUSR)
         perm = unset_bit(perm, ffi.C.kS_IWGRP)
         perm = unset_bit(perm, ffi.C.kS_IWOTH)
         eq(OK, (os_setperm('unit-test-directory/test.file', perm)))
         eq(0, os_file_is_writable('unit-test-directory/test.file'))
-        eq(OK, (os_setperm('unit-test-directory/test.file', perm_orig)))
       end)
 
       it('returns 1 if the file is writable', function()