From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 16 Jul 2013 21:49:32 +0000 (-0700)
Subject: Fix bug where insert-file-contents closes a file twice.
X-Git-Tag: emacs-24.3.90~173^2^2~42^2~45^2~387^2~1775
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=41d48a42df6ce4e5af9543f97313e256f16aa900;p=emacs.git

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 8474e8aa5e4..ab90682aa67 100644
--- 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 1b5208e5f25..b74b0ca91c2 100644
--- 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 38413f831dc..26e9e18ec9a 100644
--- 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