Merge pull request #3531 from equalsraf/tb-cleanup-os-errors

[RFC] Cleanup use of os_* functions errors and errno

src/nvim/event/socket.c

@@ -97,7 +97,7 @@ int socket_watcher_start(SocketWatcher *watcher, int backlog, socket_cb cb)
     result = uv_listen(watcher->stream, backlog, connection_cb);
   }
 
-  assert(result <= 0);  // libuv should have returned -errno or zero.
+  assert(result <= 0);  // libuv should return negative error code or zero.
   if (result < 0) {
     if (result == -EACCES) {
       // Libuv converts ENOENT to EACCES for Windows compatibility, but if

src/nvim/event/stream.c

@@ -13,7 +13,7 @@
 
 /// Sets the stream associated with `fd` to "blocking" mode.
 ///
-/// @return `0` on success, or `-errno` on failure.
+/// @return `0` on success, or libuv error code on failure.
 int stream_set_blocking(int fd, bool blocking)
 {
   // Private loop to avoid conflict with existing watcher(s):

src/nvim/fileio.c

@@ -535,11 +535,7 @@ readfile (
     if (!newfile) {
       return FAIL;
     }
-    if (perm < 0
-#ifdef ENOENT
-        && errno == ENOENT
-#endif
-        ) {
+    if (perm == UV_ENOENT) {
       /*
        * Set the 'new-file' flag, so that when the file has
        * been created by someone else, a ":w" will complain.
@@ -582,11 +578,11 @@ readfile (
       return OK;                  /* a new file is not an error */
     } else {
       filemess(curbuf, sfname, (char_u *)(
-# ifdef EFBIG
-            (errno == EFBIG) ? _("[File too big]") :
-# endif
-# ifdef EOVERFLOW
-            (errno == EOVERFLOW) ? _("[File too big]") :
+            (fd == UV_EFBIG) ? _("[File too big]") :
+# if defined(UNIX) && defined(EOVERFLOW)
+            // libuv only returns -errno in Unix and in Windows open() does not
+            // set EOVERFLOW
+            (fd == -EOVERFLOW) ? _("[File too big]") :
 # endif
             _("[Permission Denied]")), 0);
       curbuf->b_p_ro = TRUE;                  /* must use "w!" now */

src/nvim/os/fs.c

@@ -185,13 +185,13 @@ static bool is_executable_in_path(const char_u *name, char_u **abspath)
 
 /// Opens or creates a file and returns a non-negative integer representing
 /// the lowest-numbered unused file descriptor, for use in subsequent system
-/// calls (read, write, lseek, fcntl, etc.). If the operation fails, `-errno`
-/// is returned, and no file is created or modified.
+/// calls (read, write, lseek, fcntl, etc.). If the operation fails, a libuv
+/// error code is returned, and no file is created or modified.
 ///
 /// @param flags Bitwise OR of flags defined in <fcntl.h>
 /// @param mode Permissions for the newly-created file (IGNORED if 'flags' is
 ///        not `O_CREAT` or `O_TMPFILE`), subject to the current umask
-/// @return file descriptor, or negative `errno` on failure
+/// @return file descriptor, or libuv error code on failure
 int os_open(const char* path, int flags, int mode)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -204,28 +204,29 @@ int os_open(const char* path, int flags, int mode)
 
 /// Get stat information for a file.
 ///
-/// @return OK on success, FAIL if a failure occurred.
-static bool os_stat(const char *name, uv_stat_t *statbuf)
+/// @return libuv return code.
+static int os_stat(const char *name, uv_stat_t *statbuf)
   FUNC_ATTR_NONNULL_ALL
 {
   uv_fs_t request;
   int result = uv_fs_stat(&fs_loop, &request, name, NULL);
   *statbuf = request.statbuf;
   uv_fs_req_cleanup(&request);
-  return (result == kLibuvSuccess);
+  return result;
 }
 
 /// Get the file permissions for a given file.
 ///
-/// @return `-1` when `name` doesn't exist.
+/// @return libuv error code on error.
 int32_t os_getperm(const char_u *name)
   FUNC_ATTR_NONNULL_ALL
 {
   uv_stat_t statbuf;
-  if (os_stat((char *)name, &statbuf)) {
+  int stat_result = os_stat((char *)name, &statbuf);
+  if (stat_result == kLibuvSuccess) {
     return (int32_t)statbuf.st_mode;
   } else {
-    return -1;
+    return stat_result;
   }
 }
 
@@ -270,7 +271,7 @@ bool os_file_exists(const char_u *name)
   FUNC_ATTR_NONNULL_ALL
 {
   uv_stat_t statbuf;
-  return os_stat((char *)name, &statbuf);
+  return os_stat((char *)name, &statbuf) == kLibuvSuccess;
 }
 
 /// Check if a file is readable.
@@ -322,7 +323,7 @@ int os_rename(const char_u *path, const char_u *new_path)
 
 /// Make a directory.
 ///
-/// @return `0` for success, -errno for failure.
+/// @return `0` for success, libuv error code for failure.
 int os_mkdir(const char *path, int32_t mode)
   FUNC_ATTR_NONNULL_ALL
 {
@@ -342,7 +343,7 @@ int os_mkdir(const char *path, int32_t mode)
 ///                          failed to create. I.e. it will contain dir or any
 ///                          of the higher level directories.
 ///
-/// @return `0` for success, -errno for failure.
+/// @return `0` for success, libuv error code for failure.
 int os_mkdir_recurse(const char *const dir, int32_t mode,
                      char **const failed_dir)
   FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
@@ -470,7 +471,7 @@ int os_remove(const char *path)
 bool os_fileinfo(const char *path, FileInfo *file_info)
   FUNC_ATTR_NONNULL_ALL
 {
-  return os_stat(path, &(file_info->stat));
+  return os_stat(path, &(file_info->stat)) == kLibuvSuccess;
 }
 
 /// Get the file information for a given path without following links
@@ -572,7 +573,7 @@ bool os_fileid(const char *path, FileID *file_id)
   FUNC_ATTR_NONNULL_ALL
 {
   uv_stat_t statbuf;
-  if (os_stat(path, &statbuf)) {
+  if (os_stat(path, &statbuf) == kLibuvSuccess) {
     file_id->inode = statbuf.st_ino;
     file_id->device_id = statbuf.st_dev;
     return true;

src/nvim/os/fs_defs.h

@@ -21,9 +21,9 @@ typedef struct {
   uv_dirent_t ent;  ///< @private The entry information.
 } Directory;
 
-/// Function to convert -errno error to char * error description
+/// Function to convert libuv error to char * error description
 ///
-/// -errno errors are returned by a number of os functions.
+/// negative libuv error codes are returned by a number of os functions.
 #define os_strerror uv_strerror
 
 #endif  // NVIM_OS_FS_DEFS_H

src/nvim/os/os_defs.h

@@ -103,9 +103,9 @@
 # include <strings.h>
 #endif
 
-/// Function to convert -errno error to char * error description
+/// Function to convert libuv error to char * error description
 ///
-/// -errno errors are returned by a number of os functions.
+/// negative libuv error codes are returned by a number of os functions.
 #define os_strerror uv_strerror
 
 #endif  // NVIM_OS_OS_DEFS_H

src/nvim/shada.c

@@ -815,7 +815,7 @@ static ShaDaReadResult sd_reader_skip(ShaDaReadDef *const sd_reader,
 ///
 /// All arguments are passed to os_open().
 ///
-/// @return file descriptor or -errno on failure.
+/// @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
 {
@@ -825,15 +825,15 @@ open_file_start:
   fd = os_open(fname, flags, mode);
 
   if (fd < 0) {
-    if (-fd == ENOENT) {
+    if (fd == UV_ENOENT) {
       return fd;
     }
-    if (-fd == ENOMEM && !did_try_to_free) {
+    if (fd == UV_ENOMEM && !did_try_to_free) {
       try_to_free_memory();
       did_try_to_free = true;
       goto open_file_start;
     }
-    if (-fd != EEXIST) {
+    if (fd != UV_EEXIST) {
       emsg3(_(SERR "System error while opening ShaDa file %s: %s"),
             fname, os_strerror(fd));
     }
@@ -847,7 +847,7 @@ open_file_start:
 /// @param[in]   fname      File name to open.
 /// @param[out]  sd_reader  Location where reader structure will be saved.
 ///
-/// @return -errno in case of error, 0 otherwise.
+/// @return libuv error in case of error, 0 otherwise.
 static int open_shada_file_for_reading(const char *const fname,
                                        ShaDaReadDef *sd_reader)
   FUNC_ATTR_WARN_UNUSED_RESULT FUNC_ATTR_NONNULL_ALL
@@ -965,7 +965,7 @@ static int shada_read_file(const char *const file, const int flags)
   }
 
   if (of_ret != 0) {
-    if (-of_ret == ENOENT && (flags & kShaDaMissingError)) {
+    if (of_ret == UV_ENOENT && (flags & kShaDaMissingError)) {
       emsg3(_(SERR "System error while opening ShaDa file %s for reading: %s"),
             fname, os_strerror(of_ret));
     }
@@ -2964,9 +2964,9 @@ shada_write_file_open:
     fd = (intptr_t) open_file(tempname, O_CREAT|O_WRONLY|O_NOFOLLOW|O_EXCL,
                               perm);
     if (fd < 0) {
-      if (-fd == EEXIST
+      if (fd == UV_EEXIST
 #ifdef ELOOP
-          || -fd == ELOOP
+          || fd == UV_ELOOP
 #endif
           ) {
         // File already exists, try another name

test/includes/CMakeLists.txt

@@ -8,6 +8,7 @@ foreach(hfile ${PRE_HEADERS})
     OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${post_hfile}
     COMMAND ${CMAKE_C_COMPILER} -std=c99 -E -P
       ${CMAKE_CURRENT_SOURCE_DIR}/${hfile}
+      -I${LIBUV_INCLUDE_DIRS}
       -o ${CMAKE_CURRENT_BINARY_DIR}/${post_hfile})
   list(APPEND POST_HEADERS ${post_hfile})
 endforeach()

test/includes/pre/sys/errno.h

@@ -1,4 +0,0 @@
-#include <sys/errno.h>
-
-static const int kENOENT = ENOENT;
-static const int kEEXIST = EEXIST;

test/includes/pre/uv-errno.h

@@ -0,0 +1,4 @@
+#include <uv-errno.h>
+
+static const int kUV_ENOENT = UV_ENOENT;
+static const int kUV_EEXIST = UV_EEXIST;

test/unit/os/fs_spec.lua

@@ -23,7 +23,7 @@ cimport('./src/nvim/fileio.h')
 local fs = cimport('./src/nvim/os/os.h')
 cppimport('sys/stat.h')
 cppimport('sys/fcntl.h')
-cppimport('sys/errno.h')
+cppimport('uv-errno.h')
 
 local buffer = ""
 local directory = nil
@@ -233,8 +233,8 @@ describe('fs function', function()
     end
 
     describe('os_getperm', function()
-      it('returns -1 when the given file does not exist', function()
-        eq(-1, (os_getperm('non-existing-file')))
+      it('returns UV_ENOENT when the given file does not exist', function()
+        eq(ffi.C.UV_ENOENT, (os_getperm('non-existing-file')))
       end)
 
       it('returns a perm > 0 when given an existing file', function()
@@ -445,8 +445,8 @@ describe('fs function', function()
       local new_file = 'test_new_file'
       local existing_file = 'unit-test-directory/test_existing.file'
 
-      it('returns -ENOENT for O_RDWR on a non-existing file', function()
-        eq(-ffi.C.kENOENT, (os_open('non-existing-file', ffi.C.kO_RDWR, 0)))
+      it('returns UV_ENOENT for O_RDWR on a non-existing file', function()
+        eq(ffi.C.UV_ENOENT, (os_open('non-existing-file', ffi.C.kO_RDWR, 0)))
       end)
 
       it('returns non-negative for O_CREAT on a non-existing file', function()
@@ -459,9 +459,9 @@ describe('fs function', function()
         assert.is_true(0 <= (os_open(existing_file, ffi.C.kO_CREAT, 0)))
       end)
 
-      it('returns -EEXIST for O_CREAT|O_EXCL on a existing file', function()
+      it('returns UV_EEXIST for O_CREAT|O_EXCL on a existing file', function()
         assert_file_exists(existing_file)
-        eq(-ffi.C.kEEXIST, (os_open(existing_file, (bit.bor(ffi.C.kO_CREAT, ffi.C.kO_EXCL)), 0)))
+        eq(ffi.C.kUV_EEXIST, (os_open(existing_file, (bit.bor(ffi.C.kO_CREAT, ffi.C.kO_EXCL)), 0)))
       end)
 
       it('sets `rwx` permissions for O_CREAT 700', function()