From a773ed9ac83b950cb2a934e9d54e1880f44bd350 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 12 Jul 2013 10:30:48 -0700 Subject: [PATCH] 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. --- src/ChangeLog | 23 +++++++++++++++++ src/callproc.c | 74 ++++++++++++++++++++++++++--------------------------- src/fileio.c | 81 +++++++++++++++++++++++++++++----------------------------- src/image.c | 5 +--- src/lisp.h | 1 + src/lread.c | 2 +- src/process.c | 26 +++++++++---------- src/unexaix.c | 9 +++---- src/unexcoff.c | 3 ++- 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 + 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 -- 2.11.4.GIT