[Emacs-diffs] trunk r113438: Fix bug where insert-file-contents closes a file twice.

[Paul Eggert]

------------------------------------------------------------
revno: 113438
revision-id: address@hidden
parent: address@hidden
fixes bug: http://debbugs.gnu.org/14839
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Tue 2013-07-16 14:49:32 -0700
message:
  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.
modified:
  src/ChangeLog                  changelog-20091113204419-o5vbwnq5f7feedwu-1438
  src/fileio.c                   fileio.c-20091113204419-o5vbwnq5f7feedwu-210
  src/lisp.h                     lisp.h-20091113204419-o5vbwnq5f7feedwu-253

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2013-07-16 21:35:45 +0000
+++ b/src/ChangeLog     2013-07-16 21:49:32 +0000
@@ -1,5 +1,15 @@
 2013-07-16  Paul Eggert  <address@hidden>
 
+       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.

=== modified file 'src/fileio.c'
--- a/src/fileio.c      2013-07-16 21:35:45 +0000
+++ b/src/fileio.c      2013-07-16 21:49:32 +0000
@@ -215,7 +215,8 @@
 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 @@
        && 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 @@
       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 @@
            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 @@
     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 @@
 
  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)

=== modified file 'src/lisp.h'
--- a/src/lisp.h        2013-07-16 21:35:45 +0000
+++ b/src/lisp.h        2013-07-16 21:49:32 +0000
@@ -2750,6 +2750,12 @@
   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