Clean up errno reporting and fix some errno-reporting bugs.

* callproc.c (Fcall_process):
* fileio.c (Fcopy_file, Finsert_file_contents, Fwrite_region):
* process.c (create_process, Fmake_network_process):
* unexaix.c (report_error):
* unexcoff.c (report_error):
Be more careful about reporting the errno of failed operations.
The code previously reported the wrong errno sometimes.
Also, prefer report_file_errno to setting errno + report_file_error.
(Fcall_process): Look at openp return value rather than at path,
as that's a bit faster and clearer when there's a numeric predicate.
* fileio.c (report_file_errno): New function, with most of the
old contents of report_file_error.
(report_file_error): Use it.
(Ffile_exists_p, Ffile_accessible_directory_p):
Set errno to 0 when it is junk.
* fileio.c (Faccess_file):
* image.c (x_create_bitmap_from_file):
Use faccessat rather than opening the file, to avoid the hassle of
having a file descriptor open.
* lisp.h (report_file_errno): New decl.
* lread.c (Flocate_file_internal): File descriptor 0 is valid, too.

9 files changed, 120 insertions, 104 deletions

diff --git a/src/ChangeLog b/src/ChangeLog
index dfc74d7bb39..6e3a82c7c13 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,28 @@
 2013-07-12  Paul Eggert  <eggert@cs.ucla.edu>
 
+        Clean up errno reporting and fix some errno-reporting bugs.
+        * callproc.c (Fcall_process):
+        * fileio.c (Fcopy_file, Finsert_file_contents, Fwrite_region):
+        * process.c (create_process, Fmake_network_process):
+        * unexaix.c (report_error):
+        * unexcoff.c (report_error):
+        Be more careful about reporting the errno of failed operations.
+        The code previously reported the wrong errno sometimes.
+        Also, prefer report_file_errno to setting errno + report_file_error.
+        (Fcall_process): Look at openp return value rather than at path,
+        as that's a bit faster and clearer when there's a numeric predicate.
+        * fileio.c (report_file_errno): New function, with most of the
+        old contents of report_file_error.
+        (report_file_error): Use it.
+        (Ffile_exists_p, Ffile_accessible_directory_p):
+        Set errno to 0 when it is junk.
+        * fileio.c (Faccess_file):
+        * image.c (x_create_bitmap_from_file):
+        Use faccessat rather than opening the file, to avoid the hassle of
+        having a file descriptor open.
+        * lisp.h (report_file_errno): New decl.
+        * lread.c (Flocate_file_internal): File descriptor 0 is valid, too.
+
         Minor EBADF fixes.
         * process.c (create_process, wait_reading_process_output) [AIX]:
         Remove obsolete SIGHUP-related  code, as Emacs no longer disables
diff --git a/src/callproc.c b/src/callproc.c
index ac9477bff10..30f9dc58d46 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -419,9 +419,10 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
                               default_output_mode);
       if (fd_output < 0)
         {
+          int open_errno = errno;
           output_file = DECODE_FILE (output_file);
-          report_file_error ("Opening process output file",
-                             Fcons (output_file, Qnil));
+          report_file_errno ("Opening process output file",
+                             Fcons (output_file, Qnil), open_errno);
         }
       if (STRINGP (error_file) || NILP (error_file))
         output_to_buffer = 0;
@@ -430,18 +431,19 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
   /* Search for program; barf if not found.  */
   {
     struct gcpro gcpro1, gcpro2, gcpro3, gcpro4;
+    int ok;
 
     GCPRO4 (infile, buffer, current_dir, error_file);
-    openp (Vexec_path, args[0], Vexec_suffixes, &path, make_number (X_OK));
+    ok = openp (Vexec_path, args[0], Vexec_suffixes, &path, make_number (X_OK));
     UNGCPRO;
+    if (ok < 0)
+      {
+        int openp_errno = errno;
+        emacs_close (filefd);
+        report_file_errno ("Searching for program",
+                           Fcons (args[0], Qnil), openp_errno);
+      }
   }
-  if (NILP (path))
-    {
-      int openp_errno = errno;
-      emacs_close (filefd);
-      errno = openp_errno;
-      report_file_error ("Searching for program", Fcons (args[0], Qnil));
-    }
 
   /* If program file name starts with /: for quoting a magic name,
      discard that.  */
@@ -499,11 +501,13 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
       mktemp (tempfile);
       outfilefd = emacs_open (tempfile, O_WRONLY | O_CREAT | O_TRUNC,
                               S_IREAD | S_IWRITE);
-      if (outfilefd < 0) {
-        emacs_close (filefd);
-        report_file_error ("Opening process output file",
-                           Fcons (build_string (tempfile), Qnil));
-      }
+      if (outfilefd < 0)
+        {
+          int open_errno = errno;
+          emacs_close (filefd);
+          report_file_errno ("Opening process output file",
+                             Fcons (build_string (tempfile), Qnil), open_errno);
+        }
     }
   else
     outfilefd = fd_output;
@@ -524,8 +528,7 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
         {
           int pipe_errno = errno;
           emacs_close (filefd);
-          errno = pipe_errno;
-          report_file_error ("Creating process pipe", Qnil);
+          report_file_errno ("Creating process pipe", Qnil, pipe_errno);
         }
       fd0 = fd[0];
       fd1 = fd[1];
@@ -547,6 +550,7 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
 
     if (fd_error < 0)
       {
+        int open_errno = errno;
         emacs_close (filefd);
         if (fd0 != filefd)
           emacs_close (fd0);
@@ -559,7 +563,8 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
           error_file = build_string (NULL_DEVICE);
         else if (STRINGP (error_file))
           error_file = DECODE_FILE (error_file);
-        report_file_error ("Cannot redirect stderr", Fcons (error_file, Qnil));
+        report_file_errno ("Cannot redirect stderr",
+                           Fcons (error_file, Qnil), open_errno);
       }
 
 #ifdef MSDOS /* MW, July 1993 */
@@ -587,10 +592,12 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
         fd0 = emacs_open (tempfile, O_RDONLY | O_BINARY, 0);
         if (fd0 < 0)
           {
+            int open_errno = errno;
             unlink (tempfile);
             emacs_close (filefd);
-            report_file_error ("Cannot re-open temporary file",
-                               Fcons (build_string (tempfile), Qnil));
+            report_file_errno ("Cannot re-open temporary file",
+                               Fcons (build_string (tempfile), Qnil),
+                               open_errno);
           }
       }
     else
@@ -708,10 +715,7 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS)  *
   }
 
   if (pid < 0)
-    {
-      errno = child_errno;
-      report_file_error ("Doing vfork", Qnil);
-    }
+    report_file_errno ("Doing vfork", Qnil, child_errno);
 
   if (INTEGERP (buffer))
     return unbind_to (count, Qnil);
@@ -1039,7 +1043,7 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r
 
 #if defined HAVE_MKOSTEMP || defined HAVE_MKSTEMP
     {
-      int fd;
+      int fd, open_errno;
 
       block_input ();
 # ifdef HAVE_MKOSTEMP
@@ -1047,23 +1051,19 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r
 # else
       fd = mkstemp (tempfile);
 # endif
+      open_errno = errno;
       unblock_input ();
-      if (fd == -1)
-        report_file_error ("Failed to open temporary file",
-                           Fcons (build_string (tempfile), Qnil));
-      else
-        emacs_close (fd);
+      if (fd < 0)
+        report_file_errno ("Failed to open temporary file",
+                           Fcons (build_string (tempfile), Qnil), open_errno);
+      emacs_close (fd);
     }
 #else
-    errno = 0;
+    errno = EEXIST;
     mktemp (tempfile);
     if (!*tempfile)
-      {
-        if (!errno)
-          errno = EEXIST;
-        report_file_error ("Failed to open temporary file using pattern",
-                           Fcons (pattern, Qnil));
-      }
+      report_file_error ("Failed to open temporary file using pattern",
+                         Fcons (pattern, Qnil));
 #endif
 
     filename_string = build_string (tempfile);
diff --git a/src/fileio.c b/src/fileio.c
index cb863410ccf..c3566390130 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -159,11 +159,13 @@ static bool e_write (int, Lisp_Object, ptrdiff_t, ptrdiff_t,
                      struct coding_system *);
 
 
+/* Signal a file-access failure.  STRING describes the failure,
+   DATA the file that was involved, and ERRORNO the errno value.  */
+
 void
-report_file_error (const char *string, Lisp_Object data)
+report_file_errno (char const *string, Lisp_Object data, int errorno)
 {
   Lisp_Object errstring;
-  int errorno = errno;
   char *str;
 
   synchronize_system_messages_locale ();
@@ -196,6 +198,12 @@ report_file_error (const char *string, Lisp_Object data)
       }
 }
 
+void
+report_file_error (char const *string, Lisp_Object data)
+{
+  report_file_errno (string, data, errno);
+}
+
 Lisp_Object
 close_file_unwind (Lisp_Object fd)
 {
@@ -2019,11 +2027,8 @@ entries (depending on how Emacs was built).  */)
     {
       /* CopyFile doesn't set errno when it fails.  By far the most
          "popular" reason is that the target is read-only.  */
-      if (GetLastError () == 5)
-        errno = EACCES;
-      else
-        errno = EPERM;
-      report_file_error ("Copying file", Fcons (file, Fcons (newname, Qnil)));
+      report_file_errno ("Copying file", Fcons (file, Fcons (newname, Qnil)),
+                         GetLastError () == 5 ? EACCES : EPERM);
     }
   /* CopyFile retains the timestamp by default.  */
   else if (NILP (keep_time))
@@ -2084,36 +2089,25 @@ entries (depending on how Emacs was built).  */)
 
   if (out_st.st_mode != 0
       && st.st_dev == out_st.st_dev && st.st_ino == out_st.st_ino)
-    {
-      errno = 0;
-      report_file_error ("Input and output files are the same",
-                         Fcons (file, Fcons (newname, Qnil)));
-    }
+    report_file_errno ("Input and output files are the same",
+                       Fcons (file, Fcons (newname, Qnil)), 0);
 
   /* We can copy only regular files.  */
   if (!S_ISREG (st.st_mode))
-    {
-      /* Get a better looking error message. */
-      errno = S_ISDIR (st.st_mode) ? EISDIR : EINVAL;
-      report_file_error ("Non-regular file", Fcons (file, Qnil));
-    }
+    report_file_errno ("Non-regular file", Fcons (file, Qnil),
+                       S_ISDIR (st.st_mode) ? EISDIR : EINVAL);
 
-#ifdef MSDOS
-  /* System's default file type was set to binary by _fmode in emacs.c.  */
-  ofd = emacs_open (SDATA (encoded_newname),
-                    O_WRONLY | O_TRUNC | O_CREAT
-                    | (NILP (ok_if_already_exists) ? O_EXCL : 0),
-                    S_IREAD | S_IWRITE);
-#else  /* not MSDOS */
   {
-    mode_t new_mask = !NILP (preserve_uid_gid) ? 0600 : 0666;
-    new_mask &= st.st_mode;
+#ifndef MSDOS
+    int new_mask = st.st_mode & (!NILP (preserve_uid_gid) ? 0600 : 0666);
+#else
+    int new_mask = S_IREAD | S_IWRITE;
+#endif
     ofd = emacs_open (SSDATA (encoded_newname),
                       (O_WRONLY | O_TRUNC | O_CREAT
                        | (NILP (ok_if_already_exists) ? O_EXCL : 0)),
                       new_mask);
   }
-#endif /* not MSDOS */
   if (ofd < 0)
     report_file_error ("Opening output file", Fcons (newname, Qnil));
 
@@ -2609,7 +2603,11 @@ Use `file-symlink-p' to test for such links.  */)
      call the corresponding file handler.  */
   handler = Ffind_file_name_handler (absname, Qfile_exists_p);
   if (!NILP (handler))
-    return call2 (handler, Qfile_exists_p, absname);
+    {
+      Lisp_Object result = call2 (handler, Qfile_exists_p, absname);
+      errno = 0;
+      return result;
+    }
 
   absname = ENCODE_FILE (absname);
 
@@ -2706,7 +2704,6 @@ If there is no error, returns nil.  */)
   (Lisp_Object filename, Lisp_Object string)
 {
   Lisp_Object handler, encoded_filename, absname;
-  int fd;
 
   CHECK_STRING (filename);
   absname = Fexpand_file_name (filename, Qnil);
@@ -2721,10 +2718,8 @@ If there is no error, returns nil.  */)
 
   encoded_filename = ENCODE_FILE (absname);
 
-  fd = emacs_open (SSDATA (encoded_filename), O_RDONLY, 0);
-  if (fd < 0)
+  if (faccessat (AT_FDCWD, SSDATA (encoded_filename), R_OK, AT_EACCESS) != 0)
     report_file_error (SSDATA (string), Fcons (filename, Qnil));
-  emacs_close (fd);
 
   return Qnil;
 }
@@ -2833,7 +2828,11 @@ searchable directory.  */)
      call the corresponding file handler.  */
   handler = Ffind_file_name_handler (absname, Qfile_accessible_directory_p);
   if (!NILP (handler))
-    return call2 (handler, Qfile_accessible_directory_p, absname);
+    {
+      Lisp_Object r = call2 (handler, Qfile_accessible_directory_p, absname);
+      errno = 0;
+      return r;
+    }
 
   absname = ENCODE_FILE (absname);
   return file_accessible_directory_p (SSDATA (absname)) ? Qt : Qnil;
@@ -4575,8 +4574,8 @@ by calling `format-decode', which see.  */)
       && EMACS_NSECS (current_buffer->modtime) == NONEXISTENT_MODTIME_NSECS)
     {
       /* If visiting nonexistent file, return nil.  */
-      errno = save_errno;
-      report_file_error ("Opening input file", Fcons (orig_filename, Qnil));
+      report_file_errno ("Opening input file", Fcons (orig_filename, Qnil),
+                         save_errno);
     }
 
   if (read_quit)
@@ -4897,13 +4896,13 @@ This calls `write-region-annotate-functions' at the start, and
 
   if (desc < 0)
     {
+      int open_errno = errno;
 #ifdef CLASH_DETECTION
-      save_errno = errno;
       if (!auto_saving) unlock_file (lockname);
-      errno = save_errno;
 #endif /* CLASH_DETECTION */
       UNGCPRO;
-      report_file_error ("Opening output file", Fcons (filename, Qnil));
+      report_file_errno ("Opening output file", Fcons (filename, Qnil),
+                         open_errno);
     }
 
   record_unwind_protect (close_file_unwind, make_number (desc));
@@ -4913,13 +4912,13 @@ This calls `write-region-annotate-functions' at the start, and
       off_t ret = lseek (desc, offset, SEEK_SET);
       if (ret < 0)
         {
+          int lseek_errno = errno;
 #ifdef CLASH_DETECTION
-          save_errno = errno;
           if (!auto_saving) unlock_file (lockname);
-          errno = save_errno;
 #endif /* CLASH_DETECTION */
           UNGCPRO;
-          report_file_error ("Lseek error", Fcons (filename, Qnil));
+          report_file_errno ("Lseek error", Fcons (filename, Qnil),
+                             lseek_errno);
         }
     }
 
diff --git a/src/image.c b/src/image.c
index 00d1836116f..c085e6e63eb 100644
--- a/src/image.c
+++ b/src/image.c
@@ -316,7 +316,6 @@ x_create_bitmap_from_file (struct frame *f, Lisp_Object file)
   int xhot, yhot, result;
   ptrdiff_t id;
   Lisp_Object found;
-  int fd;
   char *filename;
 
   /* Look for an existing bitmap with the same name.  */
@@ -332,10 +331,8 @@ x_create_bitmap_from_file (struct frame *f, Lisp_Object file)
     }
 
   /* Search bitmap-file-path for the file, if appropriate.  */
-  fd = openp (Vx_bitmap_file_path, file, Qnil, &found, Qnil);
-  if (fd < 0)
+  if (openp (Vx_bitmap_file_path, file, Qnil, &found, make_number (R_OK)) < 0)
     return -1;
-  emacs_close (fd);
 
   filename = SSDATA (found);
 
diff --git a/src/lisp.h b/src/lisp.h
index 4af256f54b6..a54b2e07057 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3822,6 +3822,7 @@ extern Lisp_Object expand_and_dir_to_file (Lisp_Object, Lisp_Object);
 EXFUN (Fread_file_name, 6);     /* Not a normal DEFUN.  */
 extern Lisp_Object close_file_unwind (Lisp_Object);
 extern Lisp_Object restore_point_unwind (Lisp_Object);
+extern _Noreturn void report_file_errno (const char *, Lisp_Object, int);
 extern _Noreturn void report_file_error (const char *, Lisp_Object);
 extern bool internal_delete_file (Lisp_Object);
 extern Lisp_Object emacs_readlinkat (int, const char *);
diff --git a/src/lread.c b/src/lread.c
index 5729cca9560..f0423f166dd 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -1412,7 +1412,7 @@ directories, make sure the PREDICATE function returns `dir-ok' for them.  */)
 {
   Lisp_Object file;
   int fd = openp (path, filename, suffixes, &file, predicate);
-  if (NILP (predicate) && fd > 0)
+  if (NILP (predicate) && fd >= 0)
     emacs_close (fd);
   return file;
 }
diff --git a/src/process.c b/src/process.c
index 81be29082fc..4a38c47443a 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1616,6 +1616,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 {
   int inchannel, outchannel;
   pid_t pid;
+  int vfork_errno;
   int sv[2];
 #ifndef WINDOWSNT
   int wait_child_setup[2];
@@ -1656,9 +1657,10 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       forkout = sv[1];
       if (pipe2 (sv, O_CLOEXEC) != 0)
         {
+          int pipe_errno = errno;
           emacs_close (inchannel);
           emacs_close (forkout);
-          report_file_error ("Creating pipe", Qnil);
+          report_file_errno ("Creating pipe", Qnil, pipe_errno);
         }
       outchannel = sv[1];
       forkin = sv[0];
@@ -1837,6 +1839,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
 
   /* Back in the parent process.  */
 
+  vfork_errno = errno;
   XPROCESS (process)->pid = pid;
   if (pid >= 0)
     XPROCESS (process)->alive = 1;
@@ -1851,6 +1854,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
         emacs_close (forkin);
       if (forkin != forkout && forkout >= 0)
         emacs_close (forkout);
+      report_file_errno ("Doing vfork", Qnil, vfork_errno);
     }
   else
     {
@@ -1896,10 +1900,6 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       }
 #endif
     }
-
-  /* Now generate the error if vfork failed.  */
-  if (pid < 0)
-    report_file_error ("Doing vfork", Qnil);
 }
 
 void
@@ -3265,12 +3265,11 @@ usage: (make-network-process &rest ARGS)  */)
 
           len = sizeof xerrno;
           eassert (FD_ISSET (s, &fdset));
-          if (getsockopt (s, SOL_SOCKET, SO_ERROR, &xerrno, &len) == -1)
+          if (getsockopt (s, SOL_SOCKET, SO_ERROR, &xerrno, &len) < 0)
             report_file_error ("getsockopt failed", Qnil);
           if (xerrno)
-            errno = xerrno, report_file_error ("error during connect", Qnil);
-          else
-            break;
+            report_file_errno ("error during connect", Qnil, xerrno);
+          break;
         }
 #endif /* !WINDOWSNT */
 
@@ -3354,11 +3353,10 @@ usage: (make-network-process &rest ARGS)  */)
       if (is_non_blocking_client)
           return Qnil;
 
-      errno = xerrno;
-      if (is_server)
-        report_file_error ("make server process failed", contact);
-      else
-        report_file_error ("make client process failed", contact);
+      report_file_errno ((is_server
+                          ? "make server process failed"
+                          : "make client process failed"),
+                         contact, xerrno);
     }
 
   inch = s;
diff --git a/src/unexaix.c b/src/unexaix.c
index 45b3ca667b0..757ba6f51b3 100644
--- a/src/unexaix.c
+++ b/src/unexaix.c
@@ -94,13 +94,10 @@ static int pagemask;
 static _Noreturn void
 report_error (const char *file, int fd)
 {
+  int err = errno;
   if (fd)
-    {
-      int failed_errno = errno;
-      emacs_close (fd);
-      errno = failed_errno;
-    }
-  report_file_error ("Cannot unexec", Fcons (build_string (file), Qnil));
+    emacs_close (fd);
+  report_file_errno ("Cannot unexec", Fcons (build_string (file), Qnil), err);
 }
 
 #define ERROR0(msg) report_error_1 (new, msg)
diff --git a/src/unexcoff.c b/src/unexcoff.c
index 6b2a3336c8a..c467e59a665 100644
--- a/src/unexcoff.c
+++ b/src/unexcoff.c
@@ -127,9 +127,10 @@ static int pagemask;
 static void
 report_error (const char *file, int fd)
 {
+  int err = errno;
   if (fd)
     emacs_close (fd);
-  report_file_error ("Cannot unexec", Fcons (build_string (file), Qnil));
+  report_file_errno ("Cannot unexec", Fcons (build_string (file), Qnil), err);
 }
 
 #define ERROR0(msg) report_error_1 (new, msg, 0, 0); return -1