From d49d6ea9677eea1d30aae4244934b1c7336e35a3 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 21 Sep 2019 11:27:46 -0700 Subject: [PATCH] Revert too-picky file-access tests Problem reported by Andreas Schwab (Bug#37475). * doc/lispref/files.texi (Writing to Files) (Testing Accessibility, Kinds of Files): Document that accessibility and file-type predicates return nil if there is trouble determining accessibility or type. * etc/NEWS: Adjust, and list the affected primitives. * src/callproc.c (init_callproc): Go back to Ffile_exists_p. * src/fileio.c (PICKY_EACCES, file_test_errno): Remove. All uses removed. (Ffile_name_case_insensitive_p, Ffile_exists_p, Ffile_symlink_p) (Ffile_directory_p, Ffile_regular_p): Document that these functions return nil if there is trouble. (Ffile_name_case_insensitive_p, check_file_access) (Ffile_writable_p, Ffile_symlink_p, Ffile_directory_p) (Ffile_accessible_directory_p, Ffile_regular_p) * src/lread.c (Fload): Go back to treating trouble in determining the answer as if the file were missing. * src/fileio.c (Ffile_newer_than_file_p): Use file_attribute_errno not file_test_errno, since returning nil is not appropriate when there are two files to test; e.g., in the rare cases where both file timestamps have overflowed then neither t nor nil is correct. --- doc/lispref/files.texi | 23 +++++---- etc/NEWS | 13 +++-- src/callproc.c | 4 +- src/fileio.c | 108 +++++++++++------------------------------ src/lisp.h | 1 - src/lread.c | 14 ++---- 6 files changed, 54 insertions(+), 109 deletions(-) diff --git a/doc/lispref/files.texi b/doc/lispref/files.texi index fba9622fec..3746c6d2c9 100644 --- a/doc/lispref/files.texi +++ b/doc/lispref/files.texi @@ -607,8 +607,7 @@ Writing to Files @var{filename}. If that file does not exist, it is created. This function returns @code{nil}. -An error is signaled if @var{filename} specifies a nonwritable file, -or a nonexistent file in a directory where files cannot be created. +An error is signaled if you cannot write or create @var{filename}. When called from Lisp, this function is completely equivalent to: @@ -851,12 +850,13 @@ Testing Accessibility @defun file-exists-p filename This function returns @code{t} if a file named @var{filename} appears to exist. This does not mean you can necessarily read the file, only -that you can find out its attributes. (On GNU and other POSIX-like +that you can probably find out its attributes. (On GNU and other POSIX-like systems, this is true if the file exists and you have execute permission on the containing directories, regardless of the permissions of the file itself.) -If the file does not exist, this function returns @code{nil}. +If the file does not exist, or if there was trouble determining +whether the file exists, this function returns @code{nil}. Directories are files, so @code{file-exists-p} can return @code{t} when given a directory. However, because @code{file-exists-p} follows @@ -881,7 +881,7 @@ Testing Accessibility This function returns @code{t} if the file @var{filename} can be written or created by you, and @code{nil} otherwise. A file is writable if the file exists and you can write it. It is creatable if it does not exist, -but the specified directory does exist and you can write in that +but its parent directory does exist and you can write in that directory. In the example below, @file{foo} is not writable because the parent @@ -899,7 +899,7 @@ Testing Accessibility @defun file-accessible-directory-p dirname This function returns @code{t} if you have permission to open existing files in the directory whose name as a file is @var{dirname}; -otherwise (or if there is no such directory), it returns @code{nil}. +otherwise (e.g., if there is no such directory), it returns @code{nil}. The value of @var{dirname} may be either a directory name (such as @file{/foo/}) or the file name of a file which is a directory (such as @file{/foo}, without the final slash). @@ -914,8 +914,8 @@ Testing Accessibility @end defun @defun access-file filename string -This function opens file @var{filename} for reading, then closes it and -returns @code{nil}. However, if the open fails, it signals an error +If you can read @var{filename} this function returns @code{nil}; +otherwise it signals an error using @var{string} as the error message text. @end defun @@ -1011,6 +1011,7 @@ Kinds of Files the link points to is nontrivial, see below.) If the file @var{filename} is not a symbolic link, or does not exist, +or if there is trouble determining whether it is a symbolic link, @code{file-symlink-p} returns @code{nil}. Here are a few examples of using this function: @@ -1071,7 +1072,9 @@ Kinds of Files @defun file-directory-p filename This function returns @code{t} if @var{filename} is the name of an -existing directory, @code{nil} otherwise. +existing directory. It returns @code{nil} if @var{filename} does +not name a directory, or if there is trouble determining whether +it is a directory. This function follows symbolic links. @example @@ -1103,6 +1106,8 @@ Kinds of Files This function returns @code{t} if the file @var{filename} exists and is a regular file (not a directory, named pipe, terminal, or other I/O device). +It returns @code{nil} if @var{filename} does not exist or is not a regular +file, or if there is trouble determining whether it is a regular file. This function follows symbolic links. @end defun diff --git a/etc/NEWS b/etc/NEWS index cb04da189e..6a4a6e60d4 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -2029,11 +2029,14 @@ longer defaults to 'buffer-file-name'. ** File metadata primitives now signal an error if I/O, access, or other serious errors prevent them from determining the result. Formerly, these functions often (though not always) returned nil. -For example, if searching /etc/firewalld results in an I/O error, -(file-symlink-p "/etc/firewalld/firewalld.conf") now signals an error -instead of returning nil, because file-symlink-p cannot determine -whether a symbolic link exists there. These functions still behave as -before if the only problem is that the file does not exist. +For example, if there is an access error, I/O error or low-level +integer overflow when getting the attributes of a file F, +(file-attributes F) now signals an error instead of returning nil. +These functions still behave as before if the only problem is that the +file does not exist. The affected primitives are +directory-files-and-attributes, file-acl, file-attributes, file-modes, +file-newer-than-file-p, file-selinux-context, file-system-info, and +set-visited-file-modtime. --- ** The function 'eldoc-message' now accepts a single argument. diff --git a/src/callproc.c b/src/callproc.c index dbbf15c792..007465cd40 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -1567,12 +1567,12 @@ init_callproc (void) tem = Fexpand_file_name (build_string ("NEWS"), Vdata_directory); if (!NILP (Fequal (srcdir, Vinvocation_directory)) - || !file_access_p (SSDATA (tem), F_OK)) + || NILP (Ffile_exists_p (tem))) { Lisp_Object newdir; newdir = Fexpand_file_name (build_string ("../etc/"), lispdir); tem = Fexpand_file_name (build_string ("NEWS"), newdir); - if (file_access_p (SSDATA (tem), F_OK)) + if (!NILP (Ffile_exists_p (tem))) Vdata_directory = newdir; } } diff --git a/src/fileio.c b/src/fileio.c index b2896c1fe1..b510d48dba 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -253,30 +253,6 @@ file_attribute_errno (Lisp_Object file, int err) return file_metadata_errno ("Getting attributes", file, err); } -/* In theory, EACCES errors for predicates like file-readable-p should - be checked further because they may be problems with an ancestor - directory instead of with the file itself, which means that we - don't have reliable info about the requested file. In practice, - though, DOS_NT platforms set errno to EACCES for missing files like - "/var/mail", so signaling EACCES errors would be a mistake there. - So return nil for EACCES unless PICKY_EACCES, which is false by - default on DOS_NT. */ -#ifndef PICKY_EACCES -# ifdef DOS_NT -enum { PICKY_EACCES = false }; -# else -enum { PICKY_EACCES = true }; -# endif -#endif - -Lisp_Object -file_test_errno (Lisp_Object file, int err) -{ - if (!PICKY_EACCES && err == EACCES) - return Qnil; - return file_metadata_errno ("Testing file", file, err); -} - void close_file_unwind (int fd) { @@ -2453,7 +2429,9 @@ file_name_case_insensitive_err (Lisp_Object file) DEFUN ("file-name-case-insensitive-p", Ffile_name_case_insensitive_p, Sfile_name_case_insensitive_p, 1, 1, 0, doc: /* Return t if file FILENAME is on a case-insensitive filesystem. -The arg must be a string. */) +Return nil if FILENAME does not exist or is not on a case-insensitive +filesystem, or if there was trouble determining whether the filesystem +is case-insensitive. */) (Lisp_Object filename) { Lisp_Object handler; @@ -2467,19 +2445,16 @@ The arg must be a string. */) if (!NILP (handler)) return call2 (handler, Qfile_name_case_insensitive_p, filename); - /* If the file doesn't exist, move up the filesystem tree until we - reach an existing directory or the root. */ + /* If the file doesn't exist or there is trouble checking its + filesystem, move up the filesystem tree until we reach an + existing, trouble-free directory or the root. */ while (true) { int err = file_name_case_insensitive_err (filename); - switch (err) - { - case -1: return Qt; - default: return file_test_errno (filename, err); - case ENOENT: case ENOTDIR: break; - } + if (err <= 0) + return err < 0 ? Qt : Qnil; Lisp_Object parent = file_name_directory (filename); - /* Avoid infinite loop if the root is reported as non-existing + /* Avoid infinite loop if the root has trouble (impossible?). */ if (!NILP (Fstring_equal (parent, filename))) return Qnil; @@ -2739,8 +2714,7 @@ file_name_absolute_p (char const *filename) } /* Return t if FILE exists and is accessible via OPERATION and AMODE, - nil (setting errno) if not. Signal an error if the result cannot - be determined. */ + nil (setting errno) if not. */ static Lisp_Object check_file_access (Lisp_Object file, Lisp_Object operation, int amode) @@ -2758,22 +2732,13 @@ check_file_access (Lisp_Object file, Lisp_Object operation, int amode) } char *encoded_file = SSDATA (ENCODE_FILE (file)); - bool ok = file_access_p (encoded_file, amode); - if (ok) - return Qt; - int err = errno; - if (err == EROFS || err == ETXTBSY - || (PICKY_EACCES && err == EACCES && amode != F_OK - && file_access_p (encoded_file, F_OK))) - { - errno = err; - return Qnil; - } - return file_test_errno (file, err); + return file_access_p (encoded_file, amode) ? Qt : Qnil; } DEFUN ("file-exists-p", Ffile_exists_p, Sfile_exists_p, 1, 1, 0, doc: /* Return t if file FILENAME exists (whether or not you can read it). +Return nil if FILENAME does not exist, or if there was trouble +determining whether the file exists. See also `file-readable-p' and `file-attributes'. This returns nil for a symlink to a nonexistent file. Use `file-symlink-p' to test for such links. */) @@ -2834,16 +2799,7 @@ DEFUN ("file-writable-p", Ffile_writable_p, Sfile_writable_p, 1, 1, 0, should check ACLs though, which do affect this. */ return file_directory_p (encoded) ? Qt : Qnil; #else - if (file_access_p (SSDATA (encoded), W_OK | X_OK)) - return Qt; - int err = errno; - if (err == EROFS - || (err == EACCES && file_access_p (SSDATA (encoded), F_OK))) - { - errno = err; - return Qnil; - } - return file_test_errno (absname, err); + return file_access_p (SSDATA (encoded), W_OK | X_OK) ? Qt : Qnil; #endif } @@ -2919,7 +2875,8 @@ check_emacs_readlinkat (int fd, Lisp_Object file, char const *encoded_file) DEFUN ("file-symlink-p", Ffile_symlink_p, Sfile_symlink_p, 1, 1, 0, doc: /* Return non-nil if file FILENAME is the name of a symbolic link. The value is the link target, as a string. -Otherwise it returns nil. +Return nil if FILENAME does not exist or is not a symbolic link, +of there was trouble determining whether the file is a symbolic link. This function does not check whether the link target exists. */) (Lisp_Object filename) @@ -2935,12 +2892,13 @@ This function does not check whether the link target exists. */) if (!NILP (handler)) return call2 (handler, Qfile_symlink_p, filename); - return check_emacs_readlinkat (AT_FDCWD, filename, - SSDATA (ENCODE_FILE (filename))); + return emacs_readlinkat (AT_FDCWD, SSDATA (ENCODE_FILE (filename))); } DEFUN ("file-directory-p", Ffile_directory_p, Sfile_directory_p, 1, 1, 0, doc: /* Return t if FILENAME names an existing directory. +Return nil if FILENAME does not name a directory, or if there +was trouble determining whether FILENAME is a directory. Symbolic links to directories count as directories. See `file-symlink-p' to distinguish symlinks. */) (Lisp_Object filename) @@ -2953,9 +2911,7 @@ See `file-symlink-p' to distinguish symlinks. */) if (!NILP (handler)) return call2 (handler, Qfile_directory_p, absname); - if (file_directory_p (absname)) - return Qt; - return file_test_errno (absname, errno); + return file_directory_p (absname) ? Qt : Qnil; } /* Return true if FILE is a directory or a symlink to a directory. @@ -3040,12 +2996,7 @@ really is a readable and searchable directory. */) } Lisp_Object encoded_absname = ENCODE_FILE (absname); - if (file_accessible_directory_p (encoded_absname)) - return Qt; - int err = errno; - if (err == EACCES && file_access_p (SSDATA (encoded_absname), F_OK)) - return Qnil; - return file_test_errno (absname, err); + return file_accessible_directory_p (encoded_absname) ? Qt : Qnil; } /* If FILE is a searchable directory or a symlink to a @@ -3108,6 +3059,8 @@ file_accessible_directory_p (Lisp_Object file) DEFUN ("file-regular-p", Ffile_regular_p, Sfile_regular_p, 1, 1, 0, doc: /* Return t if FILENAME names a regular file. This is the sort of file that holds an ordinary stream of data bytes. +Return nil if FILENAME does not exist or is not a regular file, +or there was trouble determining whether FILENAME is a regular file. Symbolic links to regular files count as regular files. See `file-symlink-p' to distinguish symlinks. */) (Lisp_Object filename) @@ -3133,9 +3086,7 @@ See `file-symlink-p' to distinguish symlinks. */) Vw32_get_true_file_attributes = true_attributes; #endif - if (stat_result == 0) - return S_ISREG (st.st_mode) ? Qt : Qnil; - return file_test_errno (absname, errno); + return stat_result == 0 && S_ISREG (st.st_mode) ? Qt : Qnil; } DEFUN ("file-selinux-context", Ffile_selinux_context, @@ -3541,20 +3492,15 @@ otherwise, if FILE2 does not exist, the answer is t. */) { err1 = errno; if (err1 != EOVERFLOW) - return file_test_errno (absname1, err1); + return file_attribute_errno (absname1, err1); } - if (stat (SSDATA (ENCODE_FILE (absname2)), &st2) != 0) { - file_test_errno (absname2, errno); + file_attribute_errno (absname2, errno); return Qt; } - if (err1) - { - file_test_errno (absname1, err1); - eassume (false); - } + file_attribute_errno (absname1, err1); return (timespec_cmp (get_stat_mtime (&st2), get_stat_mtime (&st1)) < 0 ? Qt : Qnil); diff --git a/src/lisp.h b/src/lisp.h index b081ae1cee..e68d2732e2 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4315,7 +4315,6 @@ extern AVOID report_file_errno (const char *, Lisp_Object, int); extern AVOID report_file_error (const char *, Lisp_Object); extern AVOID report_file_notify_error (const char *, Lisp_Object); extern Lisp_Object file_attribute_errno (Lisp_Object, int); -extern Lisp_Object file_test_errno (Lisp_Object, int); extern bool internal_delete_file (Lisp_Object); extern Lisp_Object check_emacs_readlinkat (int, Lisp_Object, char const *); extern bool file_directory_p (Lisp_Object); diff --git a/src/lread.c b/src/lread.c index 4f3446b09d..151731a81d 100644 --- a/src/lread.c +++ b/src/lread.c @@ -1343,26 +1343,18 @@ Return t if the file exists and loads successfully. */) /* openp already checked for newness, no point doing it again. FIXME would be nice to get a message when openp ignores suffix order due to load_prefer_newer. */ - Lisp_Object notfound = found; if (!load_prefer_newer && is_elc) { result = stat (SSDATA (efound), &s1); - int err = errno; if (result == 0) { SSET (efound, SBYTES (efound) - 1, 0); result = stat (SSDATA (efound), &s2); - err = errno; SSET (efound, SBYTES (efound) - 1, 'c'); - if (result != 0) - notfound = Fsubstring (found, make_fixnum (0), - make_fixnum (-1)); } - if (result != 0) - file_test_errno (notfound, err); - else if (timespec_cmp (get_stat_mtime (&s1), - get_stat_mtime (&s2)) - < 0) + + if (result == 0 + && timespec_cmp (get_stat_mtime (&s1), get_stat_mtime (&s2)) < 0) { /* Make the progress messages mention that source is newer. */ newer = 1; -- 2.17.1