Fix bug where insert-file-contents closes a file twice.

* fileio.c (close_file_unwind): Don't close if FD is negative;
this can happen when unwinding a zapped file descriptor.
(Finsert_file_contents): Unwind-protect the fd before the point marker,
in case Emacs runs out of memory between the two unwind-protects.
Don't trash errno when closing FD.
Zap the FD in the specpdl when closing it, instead of deferring
the removal of the unwind-protect; this fixes a bug where a child
function unwinds the stack past us.

Fixes: debbugs:14839

diff --git a/src/ChangeLog b/src/ChangeLog
index 8474e8aa5e42b2492868751bedc9dc3435965b1a..ab90682aa672113085469d62f40e1c1328924fe8 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,15 @@
 2013-07-16  Paul Eggert  <eggert@cs.ucla.edu>
 
+       Fix bug where Emacs tries to close a file twice (Bug#14839).
+       * fileio.c (close_file_unwind): Don't close if FD is negative;
+       this can happen when unwinding a zapped file descriptor.
+       (Finsert_file_contents): Unwind-protect the fd before the point marker,
+       in case Emacs runs out of memory between the two unwind-protects.
+       Don't trash errno when closing FD.
+       Zap the FD in the specpdl when closing it, instead of deferring
+       the removal of the unwind-protect; this fixes a bug where a child
+       function unwinds the stack past us.
+
        New unwind-protect flavors to better type-check C callbacks.
        This also lessens the need to write wrappers for callbacks,
        and the need for make_save_pointer.

diff --git a/src/fileio.c b/src/fileio.c
index 1b5208e5f25b56ca332b82ac82f9c28546055d01..b74b0ca91c27caad15e1b9bde48f5d4007c2a912 100644 (file)
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -215,7 +215,8 @@ report_file_error (char const *string, Lisp_Object name)
 void
 close_file_unwind (int fd)
 {
-  emacs_close (fd);
+  if (0 <= fd)
+    emacs_close (fd);
 }
 
 /* Restore point, having saved it as a marker.  */
@@ -3514,7 +3515,7 @@ by calling `format-decode', which see.  */)
        && BEG == Z);
   Lisp_Object old_Vdeactivate_mark = Vdeactivate_mark;
   bool we_locked_file = 0;
-  bool deferred_remove_unwind_protect = 0;
+  ptrdiff_t fd_index;
 
   if (current_buffer->base_buffer && ! NILP (visit))
     error ("Cannot do file visiting in an indirect buffer");
@@ -3565,12 +3566,13 @@ by calling `format-decode', which see.  */)
       goto notfound;
     }
 
+  fd_index = SPECPDL_INDEX ();
+  record_unwind_protect_int (close_file_unwind, fd);
+
   /* Replacement should preserve point as it preserves markers.  */
   if (!NILP (replace))
     record_unwind_protect (restore_point_unwind, Fpoint_marker ());
 
-  record_unwind_protect_int (close_file_unwind, fd);
-
   if (fstat (fd, &st) != 0)
     report_file_error ("Input file status", orig_filename);
   mtime = get_stat_mtime (&st);
@@ -4015,15 +4017,10 @@ by calling `format-decode', which see.  */)
            memcpy (read_buf, coding.carryover, unprocessed);
        }
       UNGCPRO;
-      emacs_close (fd);
-
-      /* We should remove the unwind_protect calling
-        close_file_unwind, but other stuff has been added the stack,
-        so defer the removal till we reach the `handled' label.  */
-      deferred_remove_unwind_protect = 1;
-
       if (this < 0)
        report_file_error ("Read error", orig_filename);
+      emacs_close (fd);
+      set_unwind_protect_int (fd_index, -1);
 
       if (unprocessed > 0)
        {
@@ -4264,9 +4261,7 @@ by calling `format-decode', which see.  */)
     Vdeactivate_mark = Qt;
 
   emacs_close (fd);
-
-  /* Discard the unwind protect for closing the file.  */
-  specpdl_ptr--;
+  set_unwind_protect_int (fd_index, -1);
 
   if (how_much < 0)
     report_file_error ("Read error", orig_filename);
@@ -4393,11 +4388,6 @@ by calling `format-decode', which see.  */)
 
  handled:
 
-  if (deferred_remove_unwind_protect)
-    /* If requested above, discard the unwind protect for closing the
-       file.  */
-    specpdl_ptr--;
-
   if (!NILP (visit))
     {
       if (empty_undo_list_p)

diff --git a/src/lisp.h b/src/lisp.h
index 38413f831dc6765e54a74ceeb26a0c77683e3628..26e9e18ec9a180656023b896bdbb10b17c421a76 100644 (file)
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -2750,6 +2750,12 @@ 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