From f4b1eb36186a2f873d84d4c089292f9fb0394d31 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 17 Jul 2013 19:12:59 -0700 Subject: [PATCH] * charset.c: Fix file descriptor leaks and errno issues. Include . (load_charset_map_from_file): Don't leak file descriptor on error. Use plain record_xmalloc since the allocation is larger than MAX_ALLOCA; that's simpler here. Simplify test for exhaustion of entries. * eval.c (record_unwind_protect_nothing): * fileio.c (fclose_unwind): New functions. * lread.c (load_unwind): Remove. All uses replaced by fclose_unwind. The replacement doesn't block input, but that no longer seems necessary. --- src/ChangeLog | 15 +++++++++++++++ src/charset.c | 30 +++++++++++++++++++----------- src/eval.c | 21 +++++++++++++++++++++ src/fileio.c | 7 +++++++ src/lisp.h | 2 ++ src/lread.c | 15 +-------------- 6 files changed, 65 insertions(+), 25 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 189a353bde6..3f667a7bcc7 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,18 @@ +2013-07-18 Paul Eggert + + * charset.c: Fix file descriptor leaks and errno issues. + Include . + (load_charset_map_from_file): Don't leak file descriptor on error. + Use plain record_xmalloc since the allocation is larger than + MAX_ALLOCA; that's simpler here. Simplify test for exhaustion + of entries. + * eval.c (record_unwind_protect_nothing): + * fileio.c (fclose_unwind): + New functions. + * lread.c (load_unwind): Remove. All uses replaced by fclose_unwind. + The replacement doesn't block input, but that no longer seems + necessary. + 2013-07-17 Paul Eggert * lread.c: Fix file descriptor leaks and errno issues. diff --git a/src/charset.c b/src/charset.c index 6b7e81c156d..eedf65faa6c 100644 --- a/src/charset.c +++ b/src/charset.c @@ -28,6 +28,7 @@ along with GNU Emacs. If not, see . */ #define CHARSET_INLINE EXTERN_INLINE +#include #include #include #include @@ -477,7 +478,8 @@ read_hex (FILE *fp, bool *eof, bool *overflow) `file-name-handler-alist' to avoid running any Lisp code. */ static void -load_charset_map_from_file (struct charset *charset, Lisp_Object mapfile, int control_flag) +load_charset_map_from_file (struct charset *charset, Lisp_Object mapfile, + int control_flag) { unsigned min_code = CHARSET_MIN_CODE (charset); unsigned max_code = CHARSET_MAX_CODE (charset); @@ -487,21 +489,26 @@ load_charset_map_from_file (struct charset *charset, Lisp_Object mapfile, int co struct charset_map_entries *head, *entries; int n_entries; ptrdiff_t count; - USE_SAFE_ALLOCA; suffixes = list2 (build_string (".map"), build_string (".TXT")); count = SPECPDL_INDEX (); + record_unwind_protect_nothing (); specbind (Qfile_name_handler_alist, Qnil); fd = openp (Vcharset_map_path, mapfile, suffixes, NULL, Qnil); - unbind_to (count, Qnil); - if (fd < 0 - || ! (fp = fdopen (fd, "r"))) - error ("Failure in loading charset map: %s", SDATA (mapfile)); + fp = fd < 0 ? 0 : fdopen (fd, "r"); + if (!fp) + { + int open_errno = errno; + emacs_close (fd); + report_file_errno ("Loading charset map", mapfile, open_errno); + } + set_unwind_protect_ptr (count, fclose_unwind, fp); + unbind_to (count + 1, Qnil); - /* Use SAFE_ALLOCA instead of alloca, as `charset_map_entries' is + /* Use record_xmalloc, as `charset_map_entries' is large (larger than MAX_ALLOCA). */ - head = SAFE_ALLOCA (sizeof *head); + head = record_xmalloc (sizeof *head); entries = head; memset (entries, 0, sizeof (struct charset_map_entries)); @@ -530,9 +537,9 @@ load_charset_map_from_file (struct charset *charset, Lisp_Object mapfile, int co if (from < min_code || to > max_code || from > to || c > MAX_CHAR) continue; - if (n_entries > 0 && (n_entries % 0x10000) == 0) + if (n_entries == 0x10000) { - entries->next = SAFE_ALLOCA (sizeof *entries->next); + entries->next = record_xmalloc (sizeof *entries->next); entries = entries->next; memset (entries, 0, sizeof (struct charset_map_entries)); n_entries = 0; @@ -544,9 +551,10 @@ load_charset_map_from_file (struct charset *charset, Lisp_Object mapfile, int co n_entries++; } fclose (fp); + clear_unwind_protect (count); load_charset_map (charset, head, n_entries, control_flag); - SAFE_FREE (); + unbind_to (count, Qnil); } static void diff --git a/src/eval.c b/src/eval.c index a4f94ee1415..23834cb54f6 100644 --- a/src/eval.c +++ b/src/eval.c @@ -3190,6 +3190,8 @@ specbind (Lisp_Object symbol, Lisp_Object value) } } +/* Push unwind-protect entries of various types. */ + void record_unwind_protect (void (*function) (Lisp_Object), Lisp_Object arg) { @@ -3229,6 +3231,18 @@ static void do_nothing (void) {} +/* Push an unwind-protect entry that does nothing, so that + set_unwind_protect_ptr can overwrite it later. */ + +void +record_unwind_protect_nothing (void) +{ + record_unwind_protect_void (do_nothing); +} + +/* Clear the unwind-protect entry COUNT, so that it does nothing. + It need not be at the top of the stack. */ + void clear_unwind_protect (ptrdiff_t count) { @@ -3237,6 +3251,10 @@ clear_unwind_protect (ptrdiff_t count) p->unwind_void.func = do_nothing; } +/* Set the unwind-protect entry COUNT so that it invokes FUNC (ARG). + It need not be at the top of the stack. Discard the entry's + previous value without invoking it. */ + void set_unwind_protect_ptr (ptrdiff_t count, void (*func) (void *), void *arg) { @@ -3246,6 +3264,9 @@ set_unwind_protect_ptr (ptrdiff_t count, void (*func) (void *), void *arg) p->unwind_ptr.arg = arg; } +/* Pop and execute entries from the unwind-protect stack until the + depth COUNT is reached. Return VALUE. */ + Lisp_Object unbind_to (ptrdiff_t count, Lisp_Object value) { diff --git a/src/fileio.c b/src/fileio.c index 28b2dc84726..5fe359d58bb 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -220,6 +220,13 @@ close_file_unwind (int fd) emacs_close (fd); } +void +fclose_unwind (void *arg) +{ + FILE *stream = arg; + fclose (stream); +} + /* Restore point, having saved it as a marker. */ void diff --git a/src/lisp.h b/src/lisp.h index 231dbee2d21..518de9db0ff 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3743,6 +3743,7 @@ 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 record_unwind_protect_nothing (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); @@ -3827,6 +3828,7 @@ extern Lisp_Object Qfile_name_history; extern Lisp_Object expand_and_dir_to_file (Lisp_Object, Lisp_Object); EXFUN (Fread_file_name, 6); /* Not a normal DEFUN. */ extern void close_file_unwind (int); +extern void fclose_unwind (void *); extern void 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); diff --git a/src/lread.c b/src/lread.c index ee387b832c5..146543a99fd 100644 --- a/src/lread.c +++ b/src/lread.c @@ -145,7 +145,6 @@ static int read_emacs_mule_char (int, int (*) (int, Lisp_Object), static void readevalloop (Lisp_Object, FILE *, Lisp_Object, bool, Lisp_Object, Lisp_Object, Lisp_Object, Lisp_Object); -static void load_unwind (void *); /* Functions that read one byte from the current source READCHARFUN or unreads one byte. If the integer argument C is -1, it returns @@ -1317,7 +1316,7 @@ Return t if the file exists and loads successfully. */) } if (! stream) report_file_error ("Opening stdio stream", file); - set_unwind_protect_ptr (fd_index, load_unwind, stream); + set_unwind_protect_ptr (fd_index, fclose_unwind, stream); if (! NILP (Vpurify_flag)) Vpreloaded_file_list = Fcons (Fpurecopy (file), Vpreloaded_file_list); @@ -1387,18 +1386,6 @@ Return t if the file exists and loads successfully. */) return Qt; } - -static void -load_unwind (void *arg) -{ - FILE *stream = arg; - if (stream != NULL) - { - block_input (); - fclose (stream); - unblock_input (); - } -} static bool complete_filename_p (Lisp_Object pathname) -- 2.11.4.GIT