Subject: [PATCH] Improve reporting of I/O, access errors

From: Paul Eggert <[email-masked]>
Date: Thu, 12 Sep 2019 01:06:59 -0700

Attached is a patch to fix some problems Emacs has with failing to report I/O 
and access errors. It affects MS-Windows which I don't use, so I'm filing it as 
a bug report to give people a chance to review it. The main idea is to signal an 
error for I/O errors that are not tame errors. Typically ENOENT and ENOTDIR are 
tame, whereas the rest are more serious.
From b641aa0085f3ef4e79e7d72198c67c3b61db9e41 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 12 Sep 2019 00:59:03 -0700
Subject: [PATCH] Improve reporting of I/O, access errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Emacs was ignoring I/O and access errors in several places
when accessing files.  For example, it silently ignored I/O or
access errors when reading symbolic links.  Update Emacs so
that it signals serious I/O and access errors more consistently.
* doc/lispref/files.texi (File Attributes):
file-attributes returns nil if the file does not exist,
not if it cannot be accessed for other reasons (where
it might signal an error).
* src/dired.c (Ffile_attributes): Fix doc.
(file_attributes): If fstatat fails, use its errno in report.
Signal an error for serious problems.
* src/emacs.c (load_pdump_find_executable):
Use faccessat since check_executable no longer exists.
* src/fileio.c (check_file_access): New function,
which signals an error for serious problems.
(check_existing, check_writable): Use it.
(check_existing): Now accepts and returns Lisp_Object, and
takes ENCODED_FILE arg.  All callers changed.
(check_writable): New accepts and returns Lisp_Object.
All callers changed.
(file_name_case_insensitive_p): Now accepts Lisp_Object file
instead of char *decoded filename.  All callers changed.
Signal an error for serious problems.  Don’t squish a long
into an int.
(file_accessible_p): New function, for simplifying callers
and signaling errors for serious problems.
(Ffile_executable_p, Ffile_readable_p): Use it.
(Ffile_writable_p): Simplify a bit by exploiting new
check_writable API.
(emacs_readlinkat): Accept Lisp_Object instead of char const *.
New ENCODED_FILE arg.  All uses changed.  Signal error for
serious problems.
(file_directory_p): New arg ENCODED_FILE, and return Lisp_Object
instead of bool.  All callers changed.  Signal error for
serious problems.
(file_accessible_directory_p): New arg ENCODED_FILE,
and return Lisp_Object instead of bool.  All callers changed.
Signal error for serious problems.
(time_error_value): EACCES means the time value is unknown,
not nonexistent.
* src/filelock.c (current_lock_owner): Use -1 and -2 instead of
1 and 2 for special cases.  Return a positive errno value on failure.
All callers changed.
(lock_if_free): Negate sense of return value.  Return a positive
errno value on failure.  All callers changed.
(lock_file): Add a FIXME about the serious errors that go
undiagnosed.
(unlock_file, Ffile_locked_p): Signal serious failures.
* src/lread.c (openp): Be more consistent about which
errors are serious.  ENOENT and ENOTDIR are benign in
a path-search function; the other errors are more serious.
---
 doc/lispref/files.texi |   2 +-
 src/callproc.c         |   8 +-
 src/charset.c          |   2 +-
 src/dired.c            |  13 +-
 src/emacs.c            |   2 +-
 src/fileio.c           | 284 ++++++++++++++++++++++-------------------
 src/filelock.c         |  88 ++++++-------
 src/lisp.h             |   7 +-
 src/lread.c            |   8 +-
 9 files changed, 219 insertions(+), 195 deletions(-)

diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi
index 18a1f4908d..da2e1e90ae 100644
--- a/doc/lispref/files.texi
+++ b/doc/lispref/files.texi
@@ -1262,7 +1262,7 @@ File Attributes
 @defun file-attributes filename &optional id-format
 @anchor{Definition of file-attributes}
 This function returns a list of attributes of file @var{filename}.  If
-the specified file's attributes cannot be accessed, it returns @code{nil}.
+the specified file does not exist, it returns @code{nil}.
 This function does not follow symbolic links.
 The optional parameter @var{id-format} specifies the preferred format
 of attributes @acronym{UID} and @acronym{GID} (see below)---the
diff --git a/src/callproc.c b/src/callproc.c
index 4473b19a29..cc9be5661a 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -119,7 +119,7 @@ encode_current_directory (void)
   dir = expand_and_dir_to_file (dir);
   dir = ENCODE_FILE (remove_slash_colon (dir));
 
-  if (! file_accessible_directory_p (dir))
+  if (NILP (file_accessible_directory_p (curdir, dir)))
     report_file_error ("Setting current directory", curdir);
 
   return dir;
@@ -1581,12 +1581,12 @@ init_callproc (void)
   if (!will_dump_p ())
     {
       tempdir = Fdirectory_file_name (Vexec_directory);
-      if (! file_accessible_directory_p (tempdir))
+      if (NILP (file_accessible_directory_p (tempdir, tempdir)))
 	dir_warning ("arch-dependent data dir", Vexec_directory);
     }
 
   tempdir = Fdirectory_file_name (Vdata_directory);
-  if (! file_accessible_directory_p (tempdir))
+  if (NILP (file_accessible_directory_p (tempdir, tempdir)))
     dir_warning ("arch-independent data dir", Vdata_directory);
 
   sh = getenv ("SHELL");
@@ -1596,7 +1596,7 @@ init_callproc (void)
   if (PATH_GAME)
     {
       Lisp_Object path_game = build_unibyte_string (PATH_GAME);
-      if (file_accessible_directory_p (path_game))
+      if (!NILP (file_accessible_directory_p (path_game, path_game)))
 	gamedir = path_game;
     }
   Vshared_game_score_directory = gamedir;
diff --git a/src/charset.c b/src/charset.c
index 8c54381dc4..9353786cca 100644
--- a/src/charset.c
+++ b/src/charset.c
@@ -2288,7 +2288,7 @@ init_charset (void)
 {
   Lisp_Object tempdir;
   tempdir = Fexpand_file_name (build_string ("charsets"), Vdata_directory);
-  if (! file_accessible_directory_p (tempdir))
+  if (NILP (file_accessible_directory_p (tempdir, tempdir)))
     {
       /* This used to be non-fatal (dir_warning), but it should not
          happen, and if it does sooner or later it will cause some
diff --git a/src/dired.c b/src/dired.c
index cec79ab46b..3789c2d7e5 100644
--- a/src/dired.c
+++ b/src/dired.c
@@ -851,7 +851,7 @@ stat_gname (struct stat *st)
 
 DEFUN ("file-attributes", Ffile_attributes, Sfile_attributes, 1, 2, 0,
        doc: /* Return a list of attributes of file FILENAME.
-Value is nil if specified file cannot be opened.
+Value is nil if specified file does not exist.
 
 ID-FORMAT specifies the preferred format of attributes uid and gid (see
 below) - valid values are `string' and `integer'.  The latter is the
@@ -971,15 +971,18 @@ file_attributes (int fd, char const *name,
 	 information to be accurate.  */
       w32_stat_get_owner_group = 1;
 #endif
-      if (fstatat (fd, name, &s, AT_SYMLINK_NOFOLLOW) == 0)
-	err = 0;
+      err = fstatat (fd, name, &s, AT_SYMLINK_NOFOLLOW) == 0 ? 0 : errno;
 #ifdef WINDOWSNT
       w32_stat_get_owner_group = 0;
 #endif
     }
 
   if (err != 0)
-    return unbind_to (count, Qnil);
+    {
+      if (err == ENOENT || err == ENOTDIR)
+	return unbind_to (count, Qnil);
+      report_file_errno ("Getting attributes", filename, err);
+    }
 
   Lisp_Object file_type;
   if (S_ISLNK (s.st_mode))
@@ -988,7 +991,7 @@ file_attributes (int fd, char const *name,
 	 symlink is replaced between the call to fstatat and the call
 	 to emacs_readlinkat.  Detect this race unless the replacement
 	 is also a symlink.  */
-      file_type = emacs_readlinkat (fd, name);
+      file_type = emacs_readlinkat (fd, filename, name);
       if (NILP (file_type))
 	return unbind_to (count, Qnil);
     }
diff --git a/src/emacs.c b/src/emacs.c
index 5a526687b1..26306f7cfe 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -746,7 +746,7 @@ load_pdump_find_executable (char const *argv0, ptrdiff_t *candidate_size)
       candidate[path_part_length] = DIRECTORY_SEP;
       memcpy (candidate + path_part_length + 1, argv0, argv0_length + 1);
       struct stat st;
-      if (check_executable (candidate)
+      if (faccessat (AT_FDCWD, candidate, X_OK, AT_EACCESS) == 0
 	  && stat (candidate, &st) == 0 && S_ISREG (st.st_mode))
 	return candidate;
       *candidate = '\0';
diff --git a/src/fileio.c b/src/fileio.c
index cbc0c89cf3..3267ba8505 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -140,53 +140,64 @@ #define DRIVE_LETTER(x) c_tolower (x)
 		     struct coding_system *);
 
 
-/* Return true if FILENAME exists, otherwise return false and set errno.  */
+/* Test whether FILE is accessible for AMODE.
+   ENCODED_FILE is FILE, encoded for the system.
+   Return t if accessible, nil (setting errno) if access is denied.
+   Signal an error if accessibility cannot be determined.  */
 
-static bool
-check_existing (const char *filename)
+static Lisp_Object
+check_file_access (Lisp_Object file, char const *encoded_file, int amode)
 {
-  return faccessat (AT_FDCWD, filename, F_OK, AT_EACCESS) == 0;
+  if (faccessat (AT_FDCWD, encoded_file, amode, AT_EACCESS) == 0)
+    return Qt;
+  int err = errno;
+
+#ifdef CYGWIN
+  /* Return success if faccessat failed because Cygwin couldn't
+     determine the file's UID or GID.  */
+  struct stat st;
+  if (stat (encoded_file, &st) == 0 && (st.st_uid == -1 || st.st_gid == -1))
+    return Qt;
+#endif
+
+  if (err == ENOENT || err == ENOTDIR || err == EROFS || err == ETXTBSY
+      || (err == EACCES && amode != F_OK
+	  && faccessat (AT_FDCWD, encoded_file, F_OK, AT_EACCESS) == 0))
+    {
+      errno = err;
+      return Qnil;
+    }
+  report_file_errno ("Testing file", file, err);
 }
 
-/* Return true if file FILENAME exists and can be executed.  */
+/* Return t if FILE exists, nil (setting errno) if not.  ENCODED_FILE
+   is FILE, encoded for the system.  Signal an error if the result
+   cannot be determined.  */
 
-bool
-check_executable (char *filename)
+static Lisp_Object
+check_existing (Lisp_Object file, char const *encoded_file)
 {
-  return faccessat (AT_FDCWD, filename, X_OK, AT_EACCESS) == 0;
+  return check_file_access (file, encoded_file, F_OK);
 }
 
-/* Return true if file FILENAME exists and can be accessed
-   according to AMODE, which should include W_OK.
-   On failure, return false and set errno.  */
+/* Return t if FILE exists and can be accessed, nil (setting errno) if not.
+   The access mode is AMODE, which should include W_OK.
+   Signal an error if the result cannot be determined.  */
 
-static bool
-check_writable (const char *filename, int amode)
+static Lisp_Object
+check_writable (Lisp_Object file, int amode)
 {
+  char *filename = SSDATA (ENCODE_FILE (file));
 #ifdef MSDOS
   /* FIXME: an faccessat implementation should be added to the
      DOS/Windows ports and this #ifdef branch should be removed.  */
   struct stat st;
   if (stat (filename, &st) < 0)
-    return 0;
+    return Qnil;
   errno = EPERM;
-  return (st.st_mode & S_IWRITE || S_ISDIR (st.st_mode));
+  return st.st_mode & S_IWRITE || S_ISDIR (st.st_mode) ? Qt : Qnil;
 #else /* not MSDOS */
-  bool res = faccessat (AT_FDCWD, filename, amode, AT_EACCESS) == 0;
-#ifdef CYGWIN
-  /* faccessat may have returned failure because Cygwin couldn't
-     determine the file's UID or GID; if so, we return success.  */
-  if (!res)
-    {
-      int faccessat_errno = errno;
-      struct stat st;
-      if (stat (filename, &st) < 0)
-        return 0;
-      res = (st.st_uid == -1 || st.st_gid == -1);
-      errno = faccessat_errno;
-    }
-#endif /* CYGWIN */
-  return res;
+  return check_file_access (file, filename, amode);
 #endif /* not MSDOS */
 }
 
@@ -2382,7 +2393,7 @@ internal_delete_file (Lisp_Object filename)
    detect this?  */
 
 static bool
-file_name_case_insensitive_p (const char *filename)
+file_name_case_insensitive_p (Lisp_Object file)
 {
   /* Use pathconf with _PC_CASE_INSENSITIVE or _PC_CASE_SENSITIVE if
      those flags are available.  As of this writing (2017-05-20),
@@ -2390,14 +2401,19 @@ file_name_case_insensitive_p (const char *filename)
      with Cygwin-2.6.1), and macOS is the only platform known to
      support the latter.  */
 
-#ifdef _PC_CASE_INSENSITIVE
-  int res = pathconf (filename, _PC_CASE_INSENSITIVE);
+#if defined _PC_CASE_INSENSITIVE || defined _PC_CASE_SENSITIVE
+  char *filename = SSDATA (ENCODE_FILE (file));
+# ifdef _PC_CASE_INSENSITIVE
+  long int res = pathconf (filename, _PC_CASE_INSENSITIVE);
   if (res >= 0)
     return res > 0;
-#elif defined _PC_CASE_SENSITIVE
-  int res = pathconf (filename, _PC_CASE_SENSITIVE);
+# else
+  long int res = pathconf (filename, _PC_CASE_SENSITIVE);
   if (res >= 0)
     return res == 0;
+# endif
+  if (errno != EINVAL)
+    report_file_error ("Testing file", file);
 #endif
 
 #if defined CYGWIN || defined DOS_NT
@@ -2439,8 +2455,7 @@ DEFUN ("file-name-case-insensitive-p", Ffile_name_case_insensitive_p,
 	  filename = newname;
 	}
     }
-  filename = ENCODE_FILE (filename);
-  return file_name_case_insensitive_p (SSDATA (filename)) ? Qt : Qnil;
+  return file_name_case_insensitive_p (filename) ? Qt : Qnil;
 }
 
 DEFUN ("rename-file", Frename_file, Srename_file, 2, 3,
@@ -2546,7 +2561,7 @@ DEFUN ("rename-file", Frename_file, Srename_file, 2, 3,
     {
       Lisp_Object symlink_target
 	= (S_ISLNK (file_st.st_mode)
-	   ? emacs_readlinkat (AT_FDCWD, SSDATA (encoded_file))
+	   ? emacs_readlinkat (AT_FDCWD, file, SSDATA (encoded_file))
 	   : Qnil);
       if (!NILP (symlink_target))
 	Fmake_symbolic_link (symlink_target, newname, ok_if_already_exists);
@@ -2717,9 +2732,20 @@ DEFUN ("file-exists-p", Ffile_exists_p, Sfile_exists_p, 1, 1, 0,
       return result;
     }
 
-  absname = ENCODE_FILE (absname);
+  return check_existing (absname, SSDATA (ENCODE_FILE (absname)));
+}
+
+/* Return t if FILE exists and is accessible via OPERATION and AMODE,
+   nil if not.  Signal an error if the result cannot be determined.  */
 
-  return check_existing (SSDATA (absname)) ? Qt : Qnil;
+static Lisp_Object
+file_accessible_p (Lisp_Object file, Lisp_Object operation, int amode)
+{
+  Lisp_Object absname = Fexpand_file_name (file, Qnil);
+  Lisp_Object handler = Ffind_file_name_handler (absname, operation);
+  if (!NILP (handler))
+    return call2 (handler, operation, absname);
+  return check_file_access (absname, SSDATA (ENCODE_FILE (absname)), amode);
 }
 
 DEFUN ("file-executable-p", Ffile_executable_p, Sfile_executable_p, 1, 1, 0,
@@ -2729,21 +2755,7 @@ DEFUN ("file-executable-p", Ffile_executable_p, Sfile_executable_p, 1, 1, 0,
 purpose, though.)  */)
   (Lisp_Object filename)
 {
-  Lisp_Object absname;
-  Lisp_Object handler;
-
-  CHECK_STRING (filename);
-  absname = Fexpand_file_name (filename, Qnil);
-
-  /* If the file name has special constructs in it,
-     call the corresponding file name handler.  */
-  handler = Ffind_file_name_handler (absname, Qfile_executable_p);
-  if (!NILP (handler))
-    return call2 (handler, Qfile_executable_p, absname);
-
-  absname = ENCODE_FILE (absname);
-
-  return (check_executable (SSDATA (absname)) ? Qt : Qnil);
+  return file_accessible_p (filename, Qfile_executable_p, X_OK);
 }
 
 DEFUN ("file-readable-p", Ffile_readable_p, Sfile_readable_p, 1, 1, 0,
@@ -2751,28 +2763,14 @@ DEFUN ("file-readable-p", Ffile_readable_p, Sfile_readable_p, 1, 1, 0,
 See also `file-exists-p' and `file-attributes'.  */)
   (Lisp_Object filename)
 {
-  Lisp_Object absname;
-  Lisp_Object handler;
-
-  CHECK_STRING (filename);
-  absname = Fexpand_file_name (filename, Qnil);
-
-  /* If the file name has special constructs in it,
-     call the corresponding file name handler.  */
-  handler = Ffind_file_name_handler (absname, Qfile_readable_p);
-  if (!NILP (handler))
-    return call2 (handler, Qfile_readable_p, absname);
-
-  absname = ENCODE_FILE (absname);
-  return (faccessat (AT_FDCWD, SSDATA (absname), R_OK, AT_EACCESS) == 0
-	  ? Qt : Qnil);
+  return file_accessible_p (filename, Qfile_readable_p, R_OK);
 }
 
 DEFUN ("file-writable-p", Ffile_writable_p, Sfile_writable_p, 1, 1, 0,
        doc: /* Return t if file FILENAME can be written or created by you.  */)
   (Lisp_Object filename)
 {
-  Lisp_Object absname, dir, encoded;
+  Lisp_Object absname, dir;
   Lisp_Object handler;
 
   CHECK_STRING (filename);
@@ -2784,11 +2782,9 @@ DEFUN ("file-writable-p", Ffile_writable_p, Sfile_writable_p, 1, 1, 0,
   if (!NILP (handler))
     return call2 (handler, Qfile_writable_p, absname);
 
-  encoded = ENCODE_FILE (absname);
-  if (check_writable (SSDATA (encoded), W_OK))
-    return Qt;
-  if (errno != ENOENT)
-    return Qnil;
+  Lisp_Object writable = check_writable (absname, W_OK);
+  if (!NILP (writable) || errno != ENOENT)
+    return writable;
 
   dir = Ffile_name_directory (absname);
   eassert (!NILP (dir));
@@ -2796,14 +2792,13 @@ DEFUN ("file-writable-p", Ffile_writable_p, Sfile_writable_p, 1, 1, 0,
   dir = Fdirectory_file_name (dir);
 #endif /* MSDOS */
 
-  dir = ENCODE_FILE (dir);
 #ifdef WINDOWSNT
   /* The read-only attribute of the parent directory doesn't affect
      whether a file or directory can be created within it.  Some day we
      should check ACLs though, which do affect this.  */
-  return file_directory_p (dir) ? Qt : Qnil;
+  return file_directory_p (dir, ENCODE_FILE (dir));
 #else
-  return check_writable (SSDATA (dir), W_OK | X_OK) ? Qt : Qnil;
+  return check_writable (dir, W_OK | X_OK);
 #endif
 }
 
@@ -2834,19 +2829,31 @@ DEFUN ("access-file", Faccess_file, Saccess_file, 2, 2, 0,
   return Qnil;
 }
 
-/* Relative to directory FD, return the symbolic link value of FILENAME.
-   On failure, return nil.  */
+/* Relative to directory FD, return the symbolic link value of FILE.
+   If FILE is not a symbolic link, return nil.
+   ENCODED_FILE is FILE, encoded for the system.
+   Signal an error on failure.  */
 Lisp_Object
-emacs_readlinkat (int fd, char const *filename)
+emacs_readlinkat (int fd, Lisp_Object file, char const *encoded_file)
 {
   static struct allocator const emacs_norealloc_allocator =
     { xmalloc, NULL, xfree, memory_full };
   Lisp_Object val;
   char readlink_buf[1024];
-  char *buf = careadlinkat (fd, filename, readlink_buf, sizeof readlink_buf,
+  char *buf = careadlinkat (fd, encoded_file,
+			    readlink_buf, sizeof readlink_buf,
 			    &emacs_norealloc_allocator, readlinkat);
   if (!buf)
-    return Qnil;
+    {
+      if (errno == EINVAL || errno == ENOENT || errno == ENOTDIR)
+	return Qnil;
+#ifdef CYGWIN
+      /* Work around Cygwin bugs.  */
+      if (errno == EIO || errno == EACCES)
+	return Qnil;
+#endif
+      report_file_error ("Reading symbolic link", file);
+    }
 
   val = build_unibyte_string (buf);
   if (buf != readlink_buf)
@@ -2874,9 +2881,8 @@ DEFUN ("file-symlink-p", Ffile_symlink_p, Sfile_symlink_p, 1, 1, 0,
   if (!NILP (handler))
     return call2 (handler, Qfile_symlink_p, filename);
 
-  filename = ENCODE_FILE (filename);
-
-  return emacs_readlinkat (AT_FDCWD, SSDATA (filename));
+  char *encoded_filename = SSDATA (ENCODE_FILE (filename));
+  return emacs_readlinkat (AT_FDCWD, filename, encoded_filename);
 }
 
 DEFUN ("file-directory-p", Ffile_directory_p, Sfile_directory_p, 1, 1, 0,
@@ -2893,49 +2899,60 @@ DEFUN ("file-directory-p", Ffile_directory_p, Sfile_directory_p, 1, 1, 0,
   if (!NILP (handler))
     return call2 (handler, Qfile_directory_p, absname);
 
-  absname = ENCODE_FILE (absname);
-
-  return file_directory_p (absname) ? Qt : Qnil;
+  return file_directory_p (absname, ENCODE_FILE (absname));
 }
 
-/* Return true if FILE is a directory or a symlink to a directory.
-   Otherwise return false and set errno.  */
-bool
-file_directory_p (Lisp_Object file)
+/* Return t if FILE is a directory or a symlink to a directory, nil
+   (setting errno) if not.  ENCODED_FILE is FILE, encoded for the system.
+   Signal an error if the result cannot be determined.  */
+Lisp_Object
+file_directory_p (Lisp_Object file, Lisp_Object encoded_file)
 {
+  char *filename = SSDATA (encoded_file);
 #ifdef DOS_NT
   /* This is cheaper than 'stat'.  */
-  return faccessat (AT_FDCWD, SSDATA (file), D_OK, AT_EACCESS) == 0;
+  return (faccessat (AT_FDCWD, filename, D_OK, AT_EACCESS) == 0
+	  ? Qt : Qnil);
 #else
+
+  bool use_generic_code = true;
+
 # ifdef O_PATH
   /* Use O_PATH if available, as it avoids races and EOVERFLOW issues.  */
-  int fd = openat (AT_FDCWD, SSDATA (file), O_PATH | O_CLOEXEC | O_DIRECTORY);
+  int fd = openat (AT_FDCWD, filename, O_PATH | O_CLOEXEC | O_DIRECTORY);
   if (0 <= fd)
     {
       emacs_close (fd);
-      return true;
+      return Qt;
     }
-  if (errno != EINVAL)
-    return false;
-  /* O_PATH is defined but evidently this Linux kernel predates 2.6.39.
-     Fall back on generic POSIX code.  */
+  /* If errno == EINVAL, O_PATH is defined but evidently this Linux
+     kernel predates 2.6.39, so fall back on generic POSIX code.  */
+  use_generic_code = errno == EINVAL;
 # endif
-  /* Use file_accessible_directory, as it avoids stat EOVERFLOW
-     problems and could be cheaper.  However, if it fails because FILE
-     is inaccessible, fall back on stat; if the latter fails with
-     EOVERFLOW then FILE must have been a directory unless a race
-     condition occurred (a problem hard to work around portably).  */
-  if (file_accessible_directory_p (file))
-    return true;
-  if (errno != EACCES)
-    return false;
-  struct stat st;
-  if (stat (SSDATA (file), &st) != 0)
-    return errno == EOVERFLOW;
-  if (S_ISDIR (st.st_mode))
-    return true;
-  errno = ENOTDIR;
-  return false;
+
+  if (use_generic_code)
+    {
+      /* Use file_accessible_directory_p, as it avoids stat EOVERFLOW
+	 problems and could be cheaper.  However, if it fails because FILE
+	 is inaccessible, fall back on stat; if the latter fails with
+	 EOVERFLOW then FILE must have been a directory unless a race
+	 condition occurred (a problem hard to work around portably).  */
+      Lisp_Object accessible = file_accessible_directory_p (file, encoded_file);
+      if (! (NILP (accessible) && errno == EACCES))
+	return accessible;
+      struct stat st;
+      if (stat (filename, &st) == 0)
+	{
+	  errno = ENOTDIR;
+	  return S_ISDIR (st.st_mode) ? Qt : Qnil;
+	}
+      if (errno == EOVERFLOW)
+	return Qt;
+    }
+
+  if (errno == ENOENT || errno == ENOTDIR)
+    return Qnil;
+  report_file_error ("Testing file", file);
 #endif
 }
 
@@ -2976,15 +2993,15 @@ DEFUN ("file-accessible-directory-p", Ffile_accessible_directory_p,
       return r;
     }
 
-  absname = ENCODE_FILE (absname);
-  return file_accessible_directory_p (absname) ? Qt : Qnil;
+  return file_accessible_directory_p (absname, ENCODE_FILE (absname));
 }
 
-/* If FILE is a searchable directory or a symlink to a
-   searchable directory, return true.  Otherwise return
-   false and set errno to an error number.  */
-bool
-file_accessible_directory_p (Lisp_Object file)
+/* If FILE is a searchable directory or a symlink to a searchable
+   directory, return t.  If not, return nil and set errno to an error
+   number.  ENCODED_FILE is FILE, encoded for the system.  Signal an
+   error if the result cannot be determined.  */
+Lisp_Object
+file_accessible_directory_p (Lisp_Object file, Lisp_Object encoded_file)
 {
 #ifdef DOS_NT
 # ifdef WINDOWSNT
@@ -2993,18 +3010,19 @@ file_accessible_directory_p (Lisp_Object file)
      accessing "DIR/.", used below on Posix hosts, doesn't work on
      Windows, because "DIR/." is normalized to just "DIR" before
      hitting the disk.  */
-  return (SBYTES (file) == 0
-	  || w32_accessible_directory_p (SSDATA (file), SBYTES (file)));
+  return ((SBYTES (encoded_file) == 0
+	   || w32_accessible_directory_p (SSDATA (encoded_file),
+					  SBYTES (encoded_file)))
+	  ? Qt : Qnil);
 # else	/* MSDOS */
-  return file_directory_p (file);
+  return file_directory_p (file, encoded_file);
 # endif	 /* MSDOS */
 #else	 /* !DOS_NT */
   /* On POSIXish platforms, use just one system call; this avoids a
      race and is typically faster.  */
-  const char *data = SSDATA (file);
-  ptrdiff_t len = SBYTES (file);
+  const char *data = SSDATA (encoded_file);
+  ptrdiff_t len = SBYTES (encoded_file);
   char const *dir;
-  bool ok;
   int saved_errno;
   USE_SAFE_ALLOCA;
 
@@ -3029,7 +3047,7 @@ file_accessible_directory_p (Lisp_Object file)
       dir = buf;
     }
 
-  ok = check_existing (dir);
+  Lisp_Object ok = check_existing (file, dir);
   saved_errno = errno;
   SAFE_FREE ();
   errno = saved_errno;
@@ -3429,7 +3447,7 @@ DEFUN ("set-file-times", Fset_file_times, Sset_file_times, 1, 2, 0,
       {
 #ifdef MSDOS
         /* Setting times on a directory always fails.  */
-        if (file_directory_p (encoded_absname))
+        if (!NILP (file_directory_p (absname, encoded_absname)))
           return Qnil;
 #endif
         report_file_error ("Setting file times", absname);
@@ -3612,7 +3630,7 @@ file_offset (Lisp_Object val)
 static struct timespec
 time_error_value (int errnum)
 {
-  int ns = (errnum == ENOENT || errnum == EACCES || errnum == ENOTDIR
+  int ns = (errnum == ENOENT || errnum == ENOTDIR
 	    ? NONEXISTENT_MODTIME_NSECS
 	    : UNKNOWN_MODTIME_NSECS);
   return make_timespec (0, ns);
diff --git a/src/filelock.c b/src/filelock.c
index 46349a63e4..db0f87010d 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -504,9 +504,9 @@ read_lock_data (char *lfname, char lfinfo[MAX_LFINFO + 1])
 }
 
 /* Return 0 if nobody owns the lock file LFNAME or the lock is obsolete,
-   1 if another process owns it (and set OWNER (if non-null) to info),
-   2 if the current process owns it,
-   or -1 if something is wrong with the locking mechanism.  */
+   -1 if another process owns it (and set OWNER (if non-null) to info),
+   -2 if the current process owns it,
+   or an errno value if something is wrong with the locking mechanism.  */
 
 static int
 current_lock_owner (lock_info_type *owner, char *lfname)
@@ -525,23 +525,23 @@ current_lock_owner (lock_info_type *owner, char *lfname)
   /* If nonexistent lock file, all is well; otherwise, got strange error. */
   lfinfolen = read_lock_data (lfname, owner->user);
   if (lfinfolen < 0)
-    return errno == ENOENT ? 0 : -1;
+    return errno == ENOENT ? 0 : errno;
   if (MAX_LFINFO < lfinfolen)
-    return -1;
+    return ENAMETOOLONG;
   owner->user[lfinfolen] = 0;
 
-  /* Parse [email-masked]:BOOT_TIME.  If can't parse, return -1.  */
+  /* Parse [email-masked]:BOOT_TIME.  If can't parse, return EINVAL.  */
   /* The USER is everything before the last @.  */
   owner->at = at = memrchr (owner->user, '@', lfinfolen);
   if (!at)
-    return -1;
+    return EINVAL;
   owner->dot = dot = strrchr (at, '.');
   if (!dot)
-    return -1;
+    return EINVAL;
 
   /* The PID is everything from the last '.' to the ':' or equivalent.  */
   if (! c_isdigit (dot[1]))
-    return -1;
+    return EINVAL;
   errno = 0;
   pid = strtoimax (dot + 1, &owner->colon, 10);
   if (errno == ERANGE)
@@ -562,20 +562,20 @@ current_lock_owner (lock_info_type *owner, char *lfname)
 	 mistakenly transliterate ':' to U+F022 in symlink contents.
 	 See <https://bugzilla.redhat.com/show_bug.cgi?id=1384153>.  */
       if (! (boot[0] == '\200' && boot[1] == '\242'))
-	return -1;
+	return EINVAL;
       boot += 2;
       FALLTHROUGH;
     case ':':
       if (! c_isdigit (boot[0]))
-	return -1;
+	return EINVAL;
       boot_time = strtoimax (boot, &lfinfo_end, 10);
       break;
 
     default:
-      return -1;
+      return EINVAL;
     }
   if (lfinfo_end != owner->user + lfinfolen)
-    return -1;
+    return EINVAL;
 
   /* On current host?  */
   Lisp_Object system_name = Fsystem_name ();
@@ -584,22 +584,22 @@ current_lock_owner (lock_info_type *owner, char *lfname)
       && memcmp (at + 1, SSDATA (system_name), SBYTES (system_name)) == 0)
     {
       if (pid == getpid ())
-        ret = 2; /* We own it.  */
+        ret = -2; /* We own it.  */
       else if (0 < pid && pid <= TYPE_MAXIMUM (pid_t)
                && (kill (pid, 0) >= 0 || errno == EPERM)
 	       && (boot_time == 0
 		   || (boot_time <= TYPE_MAXIMUM (time_t)
 		       && within_one_second (boot_time, get_boot_time ()))))
-        ret = 1; /* An existing process on this machine owns it.  */
+        ret = -1; /* An existing process on this machine owns it.  */
       /* The owner process is dead or has a strange pid, so try to
          zap the lockfile.  */
       else
-        return unlink (lfname);
+        return unlink (lfname) < 0 ? errno : 0;
     }
   else
     { /* If we wanted to support the check for stale locks on remote machines,
          here's where we'd do it.  */
-      ret = 1;
+      ret = -1;
     }
 
   return ret;
@@ -608,30 +608,28 @@ current_lock_owner (lock_info_type *owner, char *lfname)
 
 /* Lock the lock named LFNAME if possible.
    Return 0 in that case.
-   Return positive if some other process owns the lock, and info about
+   Return negative if some other process owns the lock, and info about
      that process in CLASHER.
-   Return -1 if cannot lock for any other reason.  */
+   Return positive errno value if cannot lock for any other reason.  */
 
 static int
-lock_if_free (lock_info_type *clasher, char *lfname)
+lock_if_free (lock_info_type *clasher, char *lfname, Lisp_Object fn)
 {
   int err;
   while ((err = lock_file_1 (lfname, 0)) == EEXIST)
     {
-      switch (current_lock_owner (clasher, lfname))
+      err = current_lock_owner (clasher, lfname);
+      if (err != 0)
 	{
-	case 2:
-	  return 0;   /* We ourselves locked it.  */
-	case 1:
-	  return 1;   /* Someone else has it.  */
-	case -1:
-	  return -1;  /* current_lock_owner returned strange error.  */
+	  if (err < 0)
+	    return -2 - err; /* We locked it, or someone else has it.  */
+	  break; /* current_lock_owner returned strange error.  */
 	}
 
       /* We deleted a stale lock; try again to lock the file.  */
     }
 
-  return err ? -1 : 0;
+  return err;
 }
 
 /* lock_file locks file FN,
@@ -697,8 +695,9 @@ lock_file (Lisp_Object fn)
       /* Create the name of the lock-file for file fn */
       MAKE_LOCK_NAME (lfname, encoded_fn);
 
-      /* Try to lock the lock.  */
-      if (0 < lock_if_free (&lock_info, lfname))
+      /* Try to lock the lock.  FIXME: This ignores errors when
+	 lock_if_free returns a positive errno value.  */
+      if (lock_if_free (&lock_info, lfname, fn) < 0)
 	{
 	  /* Someone else has the lock.  Consider breaking it.  */
 	  Lisp_Object attack;
@@ -725,13 +724,16 @@ unlock_file (Lisp_Object fn)
   char *lfname;
   USE_SAFE_ALLOCA;
 
-  fn = Fexpand_file_name (fn, Qnil);
-  fn = ENCODE_FILE (fn);
+  Lisp_Object filename = Fexpand_file_name (fn, Qnil);
+  fn = ENCODE_FILE (filename);
 
   MAKE_LOCK_NAME (lfname, fn);
 
-  if (current_lock_owner (0, lfname) == 2)
-    unlink (lfname);
+  int err = current_lock_owner (0, lfname);
+  if (err == -2 && unlink (lfname) != 0 && errno != ENOENT)
+    err = errno;
+  if (0 < err)
+    report_file_errno ("Unlocking file", filename, err);
 
   SAFE_FREE ();
 }
@@ -822,17 +824,17 @@ DEFUN ("file-locked-p", Ffile_locked_p, Sfile_locked_p, 1, 1, 0,
   USE_SAFE_ALLOCA;
 
   filename = Fexpand_file_name (filename, Qnil);
-  filename = ENCODE_FILE (filename);
-
-  MAKE_LOCK_NAME (lfname, filename);
+  Lisp_Object encoded_filename = ENCODE_FILE (filename);
+  MAKE_LOCK_NAME (lfname, encoded_filename);
 
   owner = current_lock_owner (&locker, lfname);
-  if (owner <= 0)
-    ret = Qnil;
-  else if (owner == 2)
-    ret = Qt;
-  else
-    ret = make_string (locker.user, locker.at - locker.user);
+  switch (owner)
+    {
+    case -2: ret = Qt; break;
+    case -1: ret = make_string (locker.user, locker.at - locker.user); break;
+    case  0: ret = Qnil; break;
+    default: report_file_errno ("Testing file lock", filename, owner);
+    }
 
   SAFE_FREE ();
   return ret;
diff --git a/src/lisp.h b/src/lisp.h
index 024e5edb26..cb28429131 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4298,7 +4298,6 @@ XMODULE_FUNCTION (Lisp_Object o)
 
 /* Defined in fileio.c.  */
 
-extern bool check_executable (char *);
 extern char *splice_dir_file (char *, char const *, char const *);
 extern bool file_name_absolute_p (const char *);
 extern char const *get_homedir (void);
@@ -4314,9 +4313,9 @@ XMODULE_FUNCTION (Lisp_Object o)
 extern AVOID report_file_error (const char *, Lisp_Object);
 extern AVOID report_file_notify_error (const char *, Lisp_Object);
 extern bool internal_delete_file (Lisp_Object);
-extern Lisp_Object emacs_readlinkat (int, const char *);
-extern bool file_directory_p (Lisp_Object);
-extern bool file_accessible_directory_p (Lisp_Object);
+extern Lisp_Object emacs_readlinkat (int, Lisp_Object, char const *);
+extern Lisp_Object file_directory_p (Lisp_Object, Lisp_Object);
+extern Lisp_Object file_accessible_directory_p (Lisp_Object, Lisp_Object);
 extern void init_fileio (void);
 extern void syms_of_fileio (void);
 
diff --git a/src/lread.c b/src/lread.c
index 6ae7a0d8ba..bf0321b03a 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1746,18 +1746,20 @@ openp (Lisp_Object path, Lisp_Object str, Lisp_Object suffixes,
 				    AT_EACCESS)
 			 == 0)
 		  {
-		    if (file_directory_p (encoded_fn))
+		    if (!NILP (file_directory_p (string, encoded_fn)))
 		      last_errno = EISDIR;
 		    else
 		      fd = 1;
 		  }
+		else if (errno != ENOENT && errno != ENOTDIR)
+		  last_errno = errno;
 	      }
 	    else
 	      {
 		fd = emacs_open (pfn, O_RDONLY, 0);
 		if (fd < 0)
 		  {
-		    if (errno != ENOENT)
+		    if (errno != ENOENT && errno != ENOTDIR)
 		      last_errno = errno;
 		  }
 		else
@@ -4517,7 +4519,7 @@ load_path_check (Lisp_Object lpath)
       if (STRINGP (dirfile))
         {
           dirfile = Fdirectory_file_name (dirfile);
-          if (! file_accessible_directory_p (dirfile))
+          if (NILP (file_accessible_directory_p (dirfile, dirfile)))
             dir_warning ("Lisp directory", XCAR (path_tail));
         }
     }
-- 
2.21.0