From a0931322f6c257bb4a4a678f62dcb4ae3b253221 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 17 Jul 2013 10:24:54 -0700 Subject: [PATCH] * lread.c: Fix file descriptor leaks and errno issues. (Fload): Close some races that leaked fds or streams when 'load' was interrupted. (Fload, openp): Report error number of last nontrivial failure to open. ENOENT counts as trivial. * eval.c (do_nothing, clear_unwind_protect, set_unwind_protect_ptr): New functions. * fileio.c (close_file_unwind): No need to test whether FD is nonnegative, now that the function is always called with a nonnegative arg. * lisp.h (set_unwind_protect_ptr, set_unwind_protect_int): Remove. All uses replaced with ... (clear_unwind_protect, set_unwind_protect_ptr): New decls. --- src/ChangeLog | 13 +++++++ src/editfns.c | 2 +- src/eval.c | 21 +++++++++++ src/fileio.c | 7 ++-- src/lisp.h | 14 ++------ src/lread.c | 111 +++++++++++++++++++++++++++++++++------------------------- 6 files changed, 103 insertions(+), 65 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 23334449ef3..189a353bde6 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,18 @@ 2013-07-17 Paul Eggert + * lread.c: Fix file descriptor leaks and errno issues. + (Fload): Close some races that leaked fds or streams when 'load' + was interrupted. + (Fload, openp): Report error number of last nontrivial failure to open. + ENOENT counts as trivial. + * eval.c (do_nothing, clear_unwind_protect, set_unwind_protect_ptr): + New functions. + * fileio.c (close_file_unwind): No need to test whether FD is nonnegative, + now that the function is always called with a nonnegative arg. + * lisp.h (set_unwind_protect_ptr, set_unwind_protect_int): Remove. + All uses replaced with ... + (clear_unwind_protect, set_unwind_protect_ptr): New decls. + A few more minor file errno-reporting bugs. * callproc.c (Fcall_process): * doc.c (Fsnarf_documentation): diff --git a/src/editfns.c b/src/editfns.c index b8fce7c1935..a4dea203a22 100644 --- a/src/editfns.c +++ b/src/editfns.c @@ -4238,7 +4238,7 @@ usage: (format STRING &rest OBJECTS) */) else { buf = xrealloc (buf, bufsize); - set_unwind_protect_ptr (buf_save_value_index, buf); + set_unwind_protect_ptr (buf_save_value_index, xfree, buf); } p = buf + used; diff --git a/src/eval.c b/src/eval.c index 6632084146f..a4f94ee1415 100644 --- a/src/eval.c +++ b/src/eval.c @@ -3225,6 +3225,27 @@ record_unwind_protect_void (void (*function) (void)) grow_specpdl (); } +static void +do_nothing (void) +{} + +void +clear_unwind_protect (ptrdiff_t count) +{ + union specbinding *p = specpdl + count; + p->unwind_void.kind = SPECPDL_UNWIND_VOID; + p->unwind_void.func = do_nothing; +} + +void +set_unwind_protect_ptr (ptrdiff_t count, void (*func) (void *), void *arg) +{ + union specbinding *p = specpdl + count; + p->unwind_ptr.kind = SPECPDL_UNWIND_PTR; + p->unwind_ptr.func = func; + p->unwind_ptr.arg = arg; +} + Lisp_Object unbind_to (ptrdiff_t count, Lisp_Object value) { diff --git a/src/fileio.c b/src/fileio.c index 06a0db2316f..28b2dc84726 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -217,8 +217,7 @@ report_file_error (char const *string, Lisp_Object name) void close_file_unwind (int fd) { - if (0 <= fd) - emacs_close (fd); + emacs_close (fd); } /* Restore point, having saved it as a marker. */ @@ -4029,7 +4028,7 @@ by calling `format-decode', which see. */) if (this < 0) report_file_error ("Read error", orig_filename); emacs_close (fd); - set_unwind_protect_int (fd_index, -1); + clear_unwind_protect (fd_index); if (unprocessed > 0) { @@ -4270,7 +4269,7 @@ by calling `format-decode', which see. */) Vdeactivate_mark = Qt; emacs_close (fd); - set_unwind_protect_int (fd_index, -1); + clear_unwind_protect (fd_index); if (how_much < 0) report_file_error ("Read error", orig_filename); diff --git a/src/lisp.h b/src/lisp.h index 26e9e18ec9a..231dbee2d21 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -2744,18 +2744,6 @@ SPECPDL_INDEX (void) return specpdl_ptr - specpdl; } -LISP_INLINE void -set_unwind_protect_ptr (ptrdiff_t count, void *arg) -{ - specpdl[count].unwind_ptr.arg = arg; -} - -LISP_INLINE void -set_unwind_protect_int (ptrdiff_t count, int arg) -{ - specpdl[count].unwind_int.arg = arg; -} - /* Everything needed to describe an active condition case. Members are volatile if their values need to survive _longjmp when @@ -3755,6 +3743,8 @@ extern void record_unwind_protect (void (*) (Lisp_Object), Lisp_Object); extern void record_unwind_protect_int (void (*) (int), int); extern void record_unwind_protect_ptr (void (*) (void *), void *); extern void record_unwind_protect_void (void (*) (void)); +extern void clear_unwind_protect (ptrdiff_t); +extern void set_unwind_protect_ptr (ptrdiff_t, void (*) (void *), void *); extern Lisp_Object unbind_to (ptrdiff_t, Lisp_Object); extern _Noreturn void error (const char *, ...) ATTRIBUTE_FORMAT_PRINTF (1, 2); extern _Noreturn void verror (const char *, va_list) diff --git a/src/lread.c b/src/lread.c index c4bc6fda812..ee387b832c5 100644 --- a/src/lread.c +++ b/src/lread.c @@ -1040,10 +1040,12 @@ While the file is in the process of being loaded, the variable is bound to the file's name. Return t if the file exists and loads successfully. */) - (Lisp_Object file, Lisp_Object noerror, Lisp_Object nomessage, Lisp_Object nosuffix, Lisp_Object must_suffix) + (Lisp_Object file, Lisp_Object noerror, Lisp_Object nomessage, + Lisp_Object nosuffix, Lisp_Object must_suffix) { - register FILE *stream; - register int fd = -1; + FILE *stream; + int fd; + int fd_index; ptrdiff_t count = SPECPDL_INDEX (); struct gcpro gcpro1, gcpro2, gcpro3; Lisp_Object found, efound, hist_file_name; @@ -1054,7 +1056,6 @@ Return t if the file exists and loads successfully. */) Lisp_Object handler; bool safe_p = 1; const char *fmode = "r"; - Lisp_Object tmp[2]; int version; #ifdef DOS_NT @@ -1087,19 +1088,23 @@ Return t if the file exists and loads successfully. */) else file = Fsubstitute_in_file_name (file); - /* Avoid weird lossage with null string as arg, since it would try to load a directory as a Lisp file. */ - if (SBYTES (file) > 0) + if (SCHARS (file) == 0) { - ptrdiff_t size = SBYTES (file); - + fd = -1; + errno = ENOENT; + } + else + { + Lisp_Object suffixes; found = Qnil; GCPRO2 (file, found); if (! NILP (must_suffix)) { /* Don't insist on adding a suffix if FILE already ends with one. */ + ptrdiff_t size = SBYTES (file); if (size > 3 && !strcmp (SSDATA (file) + size - 3, ".el")) must_suffix = Qnil; @@ -1112,20 +1117,28 @@ Return t if the file exists and loads successfully. */) must_suffix = Qnil; } - fd = openp (Vload_path, file, - (!NILP (nosuffix) ? Qnil - : !NILP (must_suffix) ? Fget_load_suffixes () - : Fappend (2, (tmp[0] = Fget_load_suffixes (), - tmp[1] = Vload_file_rep_suffixes, - tmp))), - &found, Qnil); + if (!NILP (nosuffix)) + suffixes = Qnil; + else + { + suffixes = Fget_load_suffixes (); + if (NILP (must_suffix)) + { + Lisp_Object arg[2]; + arg[0] = suffixes; + arg[1] = Vload_file_rep_suffixes; + suffixes = Fappend (2, arg); + } + } + + fd = openp (Vload_path, file, suffixes, &found, Qnil); UNGCPRO; } if (fd == -1) { if (NILP (noerror)) - xsignal2 (Qfile_error, build_string ("Cannot open load file"), file); + report_file_error ("Cannot open load file", file); return Qnil; } @@ -1163,6 +1176,12 @@ Return t if the file exists and loads successfully. */) #endif } + if (0 <= fd) + { + fd_index = SPECPDL_INDEX (); + record_unwind_protect_int (close_file_unwind, fd); + } + /* Check if we're stuck in a recursive load cycle. 2000-09-21: It's not possible to just check for the file loaded @@ -1178,11 +1197,7 @@ Return t if the file exists and loads successfully. */) Lisp_Object tem; for (tem = Vloads_in_progress; CONSP (tem); tem = XCDR (tem)) if (!NILP (Fequal (found, XCAR (tem))) && (++load_count > 3)) - { - if (fd >= 0) - emacs_close (fd); - signal_error ("Recursive load", Fcons (found, Vloads_in_progress)); - } + signal_error ("Recursive load", Fcons (found, Vloads_in_progress)); record_unwind_protect (record_load_unwind, Vloads_in_progress); Vloads_in_progress = Fcons (found, Vloads_in_progress); } @@ -1195,9 +1210,8 @@ Return t if the file exists and loads successfully. */) /* Get the name for load-history. */ hist_file_name = (! NILP (Vpurify_flag) - ? Fconcat (2, (tmp[0] = Ffile_name_directory (file), - tmp[1] = Ffile_name_nondirectory (found), - tmp)) + ? concat2 (Ffile_name_directory (file), + Ffile_name_nondirectory (found)) : found) ; version = -1; @@ -1223,12 +1237,7 @@ Return t if the file exists and loads successfully. */) { safe_p = 0; if (!load_dangerous_libraries) - { - if (fd >= 0) - emacs_close (fd); - error ("File `%s' was not compiled in Emacs", - SDATA (found)); - } + error ("File `%s' was not compiled in Emacs", SDATA (found)); else if (!NILP (nomessage) && !force_load_messages) message_with_string ("File `%s' not compiled in Emacs", found, 1); } @@ -1274,7 +1283,10 @@ Return t if the file exists and loads successfully. */) Lisp_Object val; if (fd >= 0) - emacs_close (fd); + { + emacs_close (fd); + clear_unwind_protect (fd_index); + } val = call4 (Vload_source_file_function, found, hist_file_name, NILP (noerror) ? Qnil : Qt, (NILP (nomessage) || force_load_messages) ? Qnil : Qt); @@ -1284,26 +1296,28 @@ Return t if the file exists and loads successfully. */) GCPRO3 (file, found, hist_file_name); -#ifdef WINDOWSNT - efound = ENCODE_FILE (found); - /* If we somehow got here with fd == -2, meaning the file is deemed - to be remote, don't even try to reopen the file locally; just - force a failure instead. */ - if (fd >= 0) + if (fd < 0) { - emacs_close (fd); - stream = emacs_fopen (SSDATA (efound), fmode); + /* We somehow got here with fd == -2, meaning the file is deemed + to be remote. Don't even try to reopen the file locally; + just force a failure. */ + stream = NULL; + errno = EINVAL; } else - stream = NULL; -#else /* not WINDOWSNT */ - stream = fdopen (fd, fmode); -#endif /* not WINDOWSNT */ - if (stream == 0) { +#ifdef WINDOWSNT emacs_close (fd); - error ("Failure to create stdio stream for %s", SDATA (file)); + clear_unwind_protect (fd_index); + efound = ENCODE_FILE (found); + stream = emacs_fopen (SSDATA (efound), fmode); +#else + stream = fdopen (fd, fmode); +#endif } + if (! stream) + report_file_error ("Opening stdio stream", file); + set_unwind_protect_ptr (fd_index, load_unwind, stream); if (! NILP (Vpurify_flag)) Vpreloaded_file_list = Fcons (Fpurecopy (file), Vpreloaded_file_list); @@ -1322,7 +1336,6 @@ Return t if the file exists and loads successfully. */) message_with_string ("Loading %s...", file, 1); } - record_unwind_protect_ptr (load_unwind, stream); specbind (Qload_file_name, found); specbind (Qinhibit_file_name_operation, Qnil); specbind (Qload_in_progress, Qt); @@ -1521,7 +1534,6 @@ openp (Lisp_Object path, Lisp_Object str, Lisp_Object suffixes, if ((!NILP (handler) || !NILP (predicate)) && !NATNUMP (predicate)) { bool exists; - last_errno = ENOENT; if (NILP (predicate)) exists = !NILP (Ffile_readable_p (string)); else @@ -1576,7 +1588,10 @@ openp (Lisp_Object path, Lisp_Object str, Lisp_Object suffixes, { fd = emacs_open (pfn, O_RDONLY, 0); if (fd < 0) - last_errno = errno; + { + if (errno != ENOENT) + last_errno = errno; + } else { struct stat st; -- 2.11.4.GIT