bug#9642: closed (Re: bug#9642: move-overlay creates an empty overlay with the evaporate property)

[Troels Nielsen <bn.troels@gmail.com>]

Hi all,

I ran into this and I think I got my head around the problem with
revision 108012.

In 108012 this test case reproduces the problem reliably for me:

(let ((b (get-buffer-create "*test-11351*"))
      (o (make-overlay (point-min) (point-max))))
  (kill-buffer b)
  (move-overlay o 1 1 b))

Executing that code will freeze up my emacs solid if using a version
between 108012 and 108059.

The problem appears to have been this: Fset_marker would set the
markers buffer to null if the new buffer was dead, and then
marker_position would throw an error, but the original buffer would
then still have a reference to the now corrupt overlay in
overlays_before or overlays_after etc..

I propose the following patch, which rearranges some lines from
#108012, adds some comments and also puts in an explicit test for
moving an overlay to a dead buffer. (though because of the
rearrangements that check is probably not strictly necessary now).
The explicit test changes the behavior slightly as earlier (before
#108012) the overlay would be removed from its buffer when trying to
move it to a dead one, while now the overlay will not be touched.

I can of course not be certain that this was everything there was to
#11351, but with the patch I have as of yet to experience any freeze
or error of the kind: "Marker does not point anywhere", and the test
mentioned in the start works.

Regards
Troels

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-04-28 22:17:27 +0000
+++ src/ChangeLog	2012-04-29 14:51:48 +0000
@@ -1,3 +1,10 @@
+2012-04-29  Troels Nielsen <bn.troels@gmail.com>
+
+	* buffer.c (Fmove_overlay): Reinstate the earlier fix for
+	(Bug#9642), but explicitly check that the buffer the overlay would
+	be moved to is live and rearrange lines to make sure that errors
+	will not put the overlay in an inconsistent state.
+
 2012-04-28  Paul Eggert  <eggert@cs.ucla.edu>

 	Do not avoid creating empty evaporating overlays (Bug#9642).

=== modified file 'src/buffer.c'
--- src/buffer.c	2012-04-28 22:17:27 +0000
+++ src/buffer.c	2012-04-29 14:42:48 +0000
@@ -3679,8 +3679,9 @@
 buffer.  */)
   (Lisp_Object overlay, Lisp_Object beg, Lisp_Object end, Lisp_Object buffer)
 {
-  struct buffer *b, *ob;
+  struct buffer *b = 0, *ob = 0;
   Lisp_Object obuffer;
+  EMACS_INT n_beg, n_end, o_beg, o_end;
   int count = SPECPDL_INDEX ();

   CHECK_OVERLAY (overlay);
@@ -3690,6 +3691,9 @@
     XSETBUFFER (buffer, current_buffer);
   CHECK_BUFFER (buffer);

+  if (NILP (Fbuffer_live_p (buffer)))
+    error ("Attempt to move overlay to a dead buffer");
+
   if (MARKERP (beg)
       && ! EQ (Fmarker_buffer (beg), buffer))
     error ("Marker points into wrong buffer");
@@ -3700,9 +3704,6 @@
   CHECK_NUMBER_COERCE_MARKER (beg);
   CHECK_NUMBER_COERCE_MARKER (end);

-  if (XINT (beg) == XINT (end) && ! NILP (Foverlay_get (overlay, Qevaporate)))
-    return Fdelete_overlay (overlay);
-
   if (XINT (beg) > XINT (end))
     {
       Lisp_Object temp;
@@ -3711,50 +3712,16 @@

   specbind (Qinhibit_quit, Qt);

+  b = XBUFFER (buffer);
+
   obuffer = Fmarker_buffer (OVERLAY_START (overlay));
-  b = XBUFFER (buffer);
-  ob = BUFFERP (obuffer) ? XBUFFER (obuffer) : (struct buffer *) 0;
-
-  /* If the overlay has changed buffers, do a thorough redisplay.  */
-  if (!EQ (buffer, obuffer))
-    {
-      /* Redisplay where the overlay was.  */
-      if (!NILP (obuffer))
-	{
-	  EMACS_INT o_beg;
-	  EMACS_INT o_end;
-
-	  o_beg = OVERLAY_POSITION (OVERLAY_START (overlay));
-	  o_end = OVERLAY_POSITION (OVERLAY_END (overlay));
-
-	  modify_overlay (ob, o_beg, o_end);
-	}
-
-      /* Redisplay where the overlay is going to be.  */
-      modify_overlay (b, XINT (beg), XINT (end));
-    }
-  else
-    /* Redisplay the area the overlay has just left, or just enclosed.  */
-    {
-      EMACS_INT o_beg, o_end;
-
+  if (!NILP (obuffer))
+    {
+      /* Make notice of original overlay values */
+      ob = XBUFFER (obuffer);
       o_beg = OVERLAY_POSITION (OVERLAY_START (overlay));
       o_end = OVERLAY_POSITION (OVERLAY_END (overlay));

-      if (o_beg == XINT (beg))
-	modify_overlay (b, o_end, XINT (end));
-      else if (o_end == XINT (end))
-	modify_overlay (b, o_beg, XINT (beg));
-      else
-	{
-	  if (XINT (beg) < o_beg) o_beg = XINT (beg);
-	  if (XINT (end) > o_end) o_end = XINT (end);
-	  modify_overlay (b, o_beg, o_end);
-	}
-    }
-
-  if (!NILP (obuffer))
-    {
       ob->overlays_before
 	= unchain_overlay (ob->overlays_before, XOVERLAY (overlay));
       ob->overlays_after
@@ -3762,12 +3729,42 @@
       eassert (XOVERLAY (overlay)->next == NULL);
     }

+  /* Set the overlay boundaries, which may clip them.  */
   Fset_marker (OVERLAY_START (overlay), beg, buffer);
   Fset_marker (OVERLAY_END   (overlay), end, buffer);
-
-  /* Put the overlay on the wrong list.  */
-  end = OVERLAY_END (overlay);
-  if (OVERLAY_POSITION (end) < b->overlay_center)
+  n_beg = marker_position (OVERLAY_START (overlay));
+  n_end = marker_position (OVERLAY_END (overlay));
+
+  /* Find out how much we should redisplay */
+  if (!EQ (buffer, obuffer))
+    /* If the overlay has changed buffers, do a thorough redisplay.  */
+    {
+      /* Redisplay where the overlay was. */
+      if (!NILP (obuffer))
+        modify_overlay (ob, o_beg, o_end);
+
+      /* Redisplay where the overlay is going to be.  */
+      modify_overlay (b, n_beg, n_end);
+    }
+  else
+    /* Redisplay the area the overlay has just left, or just enclosed.  */
+    {
+      if (o_beg == n_beg)
+	modify_overlay (b, o_end, n_end);
+      else if (o_end == n_end)
+	modify_overlay (b, o_beg, n_beg);
+      else
+        modify_overlay (b, min(n_beg, o_beg), max(o_end, n_end));
+    }
+
+  /* Delete the overlay if it is empty after clipping and has the
+     evaporate property.  */
+  if (n_beg == n_end && ! NILP (Foverlay_get (overlay, Qevaporate)))
+    return unbind_to(count, Fdelete_overlay (overlay));
+
+  /* Put the overlay into the new buffer's overlay lists, first on the
+     wrong list.  */
+  if (n_end < b->overlay_center)
     {
       XOVERLAY (overlay)->next = b->overlays_after;
       b->overlays_after = XOVERLAY (overlay);