Improve reporting of I/O, access errors

Signal an error for file-oriented errors that are not tame
errors like ENOENT and ENOTDIR (Bug#37389).
Do this for primitives exposed to Lisp; the lower
level internal C API merely makes errno values available
to higher-level C code.
* doc/lispref/files.texi (Testing Accessibility)
(File Attributes, Extended Attributes): Do not say that the
functions return nil when the return value cannot be determined.
* etc/NEWS: Mention the change.
* src/dired.c (Ffile_attributes): Fix doc string confusion
about opening a file vs getting its attributes.
(file_attributes): Signal serious errors.
* src/fileio.c (check_existing, check_executable)
(check_writable): Remove.  All callers changed to use
check_file_access or file_access_p.
(file_access_p, file_metadata_errno, file_attribute_errno)
(file_test_errno, check_file_access, check_emacs_readlinkat):
New functions.
* src/fileio.c (Ffile_executable_p, Ffile_readable_p)
(Ffile_name_case_insensitive_p, Frename_file, Ffile_exists_p):
(Ffile_symlink_p, Ffile_directory_p)
(Ffile_accessible_directory_p, Ffile_regular_p)
(Ffile_selinux_context, Ffile_acl, Ffile_modes)
(Ffile_newer_than_file_p, Fset_visited_file_modtime)
(Ffile_system_info):
* src/filelock.c (unlock_file, Ffile_locked_p):
* src/lread.c (Fload):
Signal serious errors.
* src/fileio.c (Ffile_writable_p): Remove unnecessary CHECK_STRING.
(emacs_readlinkat): Now static.
* src/filelock.c (current_lock_owner, lock_if_free): Return a
positive errno on error, and the negative of the old old value
on success.  All callers changed.
* src/lread.c (openp): Propagate serious errno values to caller.

1 files changed, 44 insertions, 42 deletions

diff --git a/src/filelock.c b/src/filelock.c
index 46349a63e4a..ff25d6475de 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 USER@HOST.PID:BOOT_TIME.  If can't parse, return -1.  */
+  /* Parse USER@HOST.PID: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,9 +608,9 @@ 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)
@@ -618,20 +618,18 @@ lock_if_free (lock_info_type *clasher, char *lfname)
   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) < 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 @@ t if it is locked by you, else a string saying which user has locked it.  */)
   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;