[Emacs-diffs] master 327d251: Avoid EOVERFLOW problems with file-directory-p

[Paul Eggert]

branch: master
commit 327d251f8a857350a78029c31c7ab3f9797cc727
Author: Paul Eggert <address@hidden>
Commit: Paul Eggert <address@hidden>

    Avoid EOVERFLOW problems with file-directory-p
    
    This fixes a bug where (file-directory-p FOO) would fail if FOO
    had an inode number out of range for ‘stat’.
    * src/fileio.c (file_directory_p): Accept a Lisp string instead of
    a C string.  All callers changed.  On non-MS-Windows hosts, use
    openat with O_PATH|O_DIRECTORY if available, otherwise
    file_accessible_directory_p unless it fails due to EACCESS,
    otherwise stat.
---
 src/fileio.c | 46 +++++++++++++++++++++++++++++++++++++---------
 src/lisp.h   |  2 +-
 src/lread.c  |  2 +-
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index 62f641f..be29e60 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -139,7 +139,7 @@ static bool e_write (int, Lisp_Object, ptrdiff_t, ptrdiff_t,
                     struct coding_system *);
 
 
-/* Return true if FILENAME exists.  */
+/* Return true if FILENAME exists, otherwise return false and set errno.  */
 
 static bool
 check_existing (const char *filename)
@@ -2595,7 +2595,7 @@ DEFUN ("file-writable-p", Ffile_writable_p, 
Sfile_writable_p, 1, 1, 0,
   /* 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 (SSDATA (dir)) ? Qt : Qnil;
+  return file_directory_p (dir) ? Qt : Qnil;
 #else
   return check_writable (SSDATA (dir), W_OK | X_OK) ? Qt : Qnil;
 #endif
@@ -2689,19 +2689,47 @@ See `file-symlink-p' to distinguish symlinks.  */)
 
   absname = ENCODE_FILE (absname);
 
-  return file_directory_p (SSDATA (absname)) ? Qt : Qnil;
+  return file_directory_p (absname) ? Qt : Qnil;
 }
 
-/* Return true if FILE is a directory or a symlink to a directory.  */
+/* Return true if FILE is a directory or a symlink to a directory.
+   Otherwise return false and set errno.  */
 bool
-file_directory_p (char const *file)
+file_directory_p (Lisp_Object file)
 {
 #ifdef WINDOWSNT
   /* This is cheaper than 'stat'.  */
-  return faccessat (AT_FDCWD, file, D_OK, AT_EACCESS) == 0;
+  return faccessat (AT_FDCWD, SSDATA (file), D_OK, AT_EACCESS) == 0;
 #else
+# 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);
+  if (0 <= fd)
+    {
+      emacs_close (fd);
+      return true;
+    }
+  if (errno != EINVAL)
+    return false;
+  /* O_PATH is defined but evidently this Linux kernel predates 2.6.39.
+     Fall back on generic POSIX code.  */
+# 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;
-  return stat (file, &st) == 0 && S_ISDIR (st.st_mode);
+  if (stat (SSDATA (file), &st) != 0)
+    return errno == EOVERFLOW;
+  if (S_ISDIR (st.st_mode))
+    return true;
+  errno = ENOTDIR;
+  return false;
 #endif
 }
 
@@ -2762,7 +2790,7 @@ file_accessible_directory_p (Lisp_Object file)
   return (SBYTES (file) == 0
          || w32_accessible_directory_p (SSDATA (file), SBYTES (file)));
 # else /* MSDOS */
-  return file_directory_p (SSDATA (file));
+  return file_directory_p (file);
 # endif         /* MSDOS */
 #else   /* !DOS_NT */
   /* On POSIXish platforms, use just one system call; this avoids a
@@ -3192,7 +3220,7 @@ Use the current time if TIMESTAMP is nil.  TIMESTAMP is 
in the format of
       {
 #ifdef MSDOS
         /* Setting times on a directory always fails.  */
-        if (file_directory_p (SSDATA (encoded_absname)))
+        if (file_directory_p (encoded_absname))
           return Qnil;
 #endif
         report_file_error ("Setting file times", absname);
diff --git a/src/lisp.h b/src/lisp.h
index d547c0c..a7f0a1d 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4130,7 +4130,7 @@ extern _Noreturn void report_file_error (const char *, 
Lisp_Object);
 extern _Noreturn void 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 (const char *);
+extern bool file_directory_p (Lisp_Object);
 extern bool file_accessible_directory_p (Lisp_Object);
 extern void init_fileio (void);
 extern void syms_of_fileio (void);
diff --git a/src/lread.c b/src/lread.c
index 1221dc9..7cacd47 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1702,7 +1702,7 @@ openp (Lisp_Object path, Lisp_Object str, Lisp_Object 
suffixes,
                                      AT_EACCESS)
                           == 0)
                    {
-                     if (file_directory_p (pfn))
+                     if (file_directory_p (encoded_fn))
                        last_errno = EISDIR;
                      else
                        fd = 1;