To: vim-dev@vim.org
Subject: About patch 7.1.130
Fcc: outbox
From: Bram Moolenaar <Bram@moolenaar.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit
------------
 
Patch 7.1.130
Problem:    Crash with specific order of undo and redo. (A.Politz)
Solution:   Clear and adjust pointers properly.  Add u_check() for debugging.
Files:	    src/undo.c, src/structs.h
 
 
*** ../vim-7.1.129/src/undo.c	Thu May 10 20:01:43 2007
--- src/undo.c	Mon Oct  1 22:49:16 2007
***************
*** 76,81 ****
--- 76,87 ----
   * buffer is unloaded.
   */
  
+ /* Uncomment the next line for including the u_check() function.  This warns
+  * for errors in the debug information. */
+ /* #define U_DEBUG 1 */
+ #define UH_MAGIC 0x18dade	/* value for uh_magic when in use */
+ #define UE_MAGIC 0xabc123	/* value for ue_magic when in use */
+ 
  #include "vim.h"
  
  /* See below: use malloc()/free() for memory management. */
***************
*** 113,118 ****
--- 119,213 ----
   */
  static int	undo_undoes = FALSE;
  
+ #ifdef U_DEBUG
+ /*
+  * Check the undo structures for being valid.  Print a warning when something
+  * looks wrong.
+  */
+ static int seen_b_u_curhead;
+ static int seen_b_u_newhead;
+ static int header_count;
+ 
+     static void
+ u_check_tree(u_header_T *uhp,
+ 	u_header_T *exp_uh_next,
+ 	u_header_T *exp_uh_alt_prev)
+ {
+     u_entry_T *uep;
+ 
+     if (uhp == NULL)
+ 	return;
+     ++header_count;
+     if (uhp == curbuf->b_u_curhead && ++seen_b_u_curhead > 1)
+     {
+ 	EMSG("b_u_curhead found twice (looping?)");
+ 	return;
+     }
+     if (uhp == curbuf->b_u_newhead && ++seen_b_u_newhead > 1)
+     {
+ 	EMSG("b_u_newhead found twice (looping?)");
+ 	return;
+     }
+ 
+     if (uhp->uh_magic != UH_MAGIC)
+ 	EMSG("uh_magic wrong (may be using freed memory)");
+     else
+     {
+ 	/* Check pointers back are correct. */
+ 	if (uhp->uh_next != exp_uh_next)
+ 	{
+ 	    EMSG("uh_next wrong");
+ 	    smsg((char_u *)"expected: 0x%x, actual: 0x%x",
+ 						   exp_uh_next, uhp->uh_next);
+ 	}
+ 	if (uhp->uh_alt_prev != exp_uh_alt_prev)
+ 	{
+ 	    EMSG("uh_alt_prev wrong");
+ 	    smsg((char_u *)"expected: 0x%x, actual: 0x%x",
+ 					   exp_uh_alt_prev, uhp->uh_alt_prev);
+ 	}
+ 
+ 	/* Check the undo tree at this header. */
+ 	for (uep = uhp->uh_entry; uep != NULL; uep = uep->ue_next)
+ 	{
+ 	    if (uep->ue_magic != UE_MAGIC)
+ 	    {
+ 		EMSG("ue_magic wrong (may be using freed memory)");
+ 		break;
+ 	    }
+ 	}
+ 
+ 	/* Check the next alt tree. */
+ 	u_check_tree(uhp->uh_alt_next, uhp->uh_next, uhp);
+ 
+ 	/* Check the next header in this branch. */
+ 	u_check_tree(uhp->uh_prev, uhp, NULL);
+     }
+ }
+ 
+     void
+ u_check(int newhead_may_be_NULL)
+ {
+     seen_b_u_newhead = 0;
+     seen_b_u_curhead = 0;
+     header_count = 0;
+ 
+     u_check_tree(curbuf->b_u_oldhead, NULL, NULL);
+ 
+     if (seen_b_u_newhead == 0 && curbuf->b_u_oldhead != NULL
+ 	    && !(newhead_may_be_NULL && curbuf->b_u_newhead == NULL))
+ 	EMSGN("b_u_newhead invalid: 0x%x", curbuf->b_u_newhead);
+     if (curbuf->b_u_curhead != NULL && seen_b_u_curhead == 0)
+ 	EMSGN("b_u_curhead invalid: 0x%x", curbuf->b_u_curhead);
+     if (header_count != curbuf->b_u_numhead)
+     {
+ 	EMSG("b_u_numhead invalid");
+ 	smsg((char_u *)"expected: %ld, actual: %ld",
+ 			       (long)header_count, (long)curbuf->b_u_numhead);
+     }
+ }
+ #endif
+ 
  /*
   * Save the current line for both the "u" and "U" command.
   * Returns OK or FAIL.
***************
*** 243,248 ****
--- 338,346 ----
      if (!undo_allowed())
  	return FAIL;
  
+ #ifdef U_DEBUG
+     u_check(FALSE);
+ #endif
  #ifdef FEAT_NETBEANS_INTG
      /*
       * Netbeans defines areas that cannot be modified.  Bail out here when
***************
*** 294,299 ****
--- 392,400 ----
  	    uhp = (u_header_T *)U_ALLOC_LINE((unsigned)sizeof(u_header_T));
  	    if (uhp == NULL)
  		goto nomem;
+ #ifdef U_DEBUG
+ 	    uhp->uh_magic = UH_MAGIC;
+ #endif
  	}
  	else
  	    uhp = NULL;
***************
*** 316,323 ****
  	{
  	    u_header_T	    *uhfree = curbuf->b_u_oldhead;
  
! 	    /* If there is no branch only free one header. */
! 	    if (uhfree->uh_alt_next == NULL)
  		u_freeheader(curbuf, uhfree, &old_curhead);
  	    else
  	    {
--- 417,427 ----
  	{
  	    u_header_T	    *uhfree = curbuf->b_u_oldhead;
  
! 	    if (uhfree == old_curhead)
! 		/* Can't reconnect the branch, delete all of it. */
! 		u_freebranch(curbuf, uhfree, &old_curhead);
! 	    else if (uhfree->uh_alt_next == NULL)
! 		/* There is no branch, only free one header. */
  		u_freeheader(curbuf, uhfree, &old_curhead);
  	    else
  	    {
***************
*** 326,331 ****
--- 430,438 ----
  		    uhfree = uhfree->uh_alt_next;
  		u_freebranch(curbuf, uhfree, &old_curhead);
  	    }
+ #ifdef U_DEBUG
+ 	    u_check(TRUE);
+ #endif
  	}
  
  	if (uhp == NULL)		/* no undo at all */
***************
*** 478,483 ****
--- 585,593 ----
      uep = (u_entry_T *)U_ALLOC_LINE((unsigned)sizeof(u_entry_T));
      if (uep == NULL)
  	goto nomem;
+ #ifdef U_DEBUG
+     uep->ue_magic = UE_MAGIC;
+ #endif
  
      uep->ue_size = size;
      uep->ue_top = top;
***************
*** 525,530 ****
--- 635,643 ----
      curbuf->b_u_synced = FALSE;
      undo_undoes = FALSE;
  
+ #ifdef U_DEBUG
+     u_check(FALSE);
+ #endif
      return OK;
  
  nomem:
***************
*** 955,960 ****
--- 1068,1076 ----
      int		empty_buffer;		    /* buffer became empty */
      u_header_T	*curhead = curbuf->b_u_curhead;
  
+ #ifdef U_DEBUG
+     u_check(FALSE);
+ #endif
      old_flags = curhead->uh_flags;
      new_flags = (curbuf->b_changed ? UH_CHANGED : 0) +
  	       ((curbuf->b_ml.ml_flags & ML_EMPTY) ? UH_EMPTYBUF : 0);
***************
*** 1186,1191 ****
--- 1302,1310 ----
      /* The timestamp can be the same for multiple changes, just use the one of
       * the undone/redone change. */
      curbuf->b_u_seq_time = curhead->uh_time;
+ #ifdef U_DEBUG
+     u_check(FALSE);
+ #endif
  }
  
  /*
***************
*** 1515,1521 ****
  }
  
  /*
!  * Free one header and its entry list and adjust the pointers.
   */
      static void
  u_freeheader(buf, uhp, uhpp)
--- 1634,1640 ----
  }
  
  /*
!  * Free one header "uhp" and its entry list and adjust the pointers.
   */
      static void
  u_freeheader(buf, uhp, uhpp)
***************
*** 1523,1528 ****
--- 1642,1649 ----
      u_header_T	    *uhp;
      u_header_T	    **uhpp;	/* if not NULL reset when freeing this header */
  {
+     u_header_T	    *uhap;
+ 
      /* When there is an alternate redo list free that branch completely,
       * because we can never go there. */
      if (uhp->uh_alt_next != NULL)
***************
*** 1540,1546 ****
      if (uhp->uh_prev == NULL)
  	buf->b_u_newhead = uhp->uh_next;
      else
! 	uhp->uh_prev->uh_next = uhp->uh_next;
  
      u_freeentries(buf, uhp, uhpp);
  }
--- 1661,1668 ----
      if (uhp->uh_prev == NULL)
  	buf->b_u_newhead = uhp->uh_next;
      else
! 	for (uhap = uhp->uh_prev; uhap != NULL; uhap = uhap->uh_alt_next)
! 	    uhap->uh_next = uhp->uh_next;
  
      u_freeentries(buf, uhp, uhpp);
  }
***************
*** 1585,1590 ****
--- 1707,1714 ----
      /* Check for pointers to the header that become invalid now. */
      if (buf->b_u_curhead == uhp)
  	buf->b_u_curhead = NULL;
+     if (buf->b_u_newhead == uhp)
+ 	buf->b_u_newhead = NULL;  /* freeing the newest entry */
      if (uhpp != NULL && uhp == *uhpp)
  	*uhpp = NULL;
  
***************
*** 1594,1599 ****
--- 1718,1726 ----
  	u_freeentry(uep, uep->ue_size);
      }
  
+ #ifdef U_DEBUG
+     uhp->uh_magic = 0;
+ #endif
      U_FREE_LINE((char_u *)uhp);
      --buf->b_u_numhead;
  }
***************
*** 1609,1614 ****
--- 1736,1744 ----
      while (n > 0)
  	U_FREE_LINE(uep->ue_array[--n]);
      U_FREE_LINE((char_u *)uep->ue_array);
+ #ifdef U_DEBUG
+     uep->ue_magic = 0;
+ #endif
      U_FREE_LINE((char_u *)uep);
  }
  
*** ../vim-7.1.129/src/structs.h	Sun Aug 12 15:50:26 2007
--- src/structs.h	Sat Sep 29 15:03:38 2007
***************
*** 278,283 ****
--- 278,286 ----
      linenr_T	ue_lcount;	/* linecount when u_save called */
      char_u	**ue_array;	/* array of lines in undo block */
      long	ue_size;	/* number of lines in ue_array */
+ #ifdef U_DEBUG
+     int		ue_magic;	/* magic number to check allocation */
+ #endif
  };
  
  struct u_header
***************
*** 300,305 ****
--- 303,311 ----
      visualinfo_T uh_visual;	/* Visual areas before undo/after redo */
  #endif
      time_t	uh_time;	/* timestamp when the change was made */
+ #ifdef U_DEBUG
+     int		uh_magic;	/* magic number to check allocation */
+ #endif
  };
  
  /* values for uh_flags */
*** ../vim-7.1.129/src/version.c	Mon Oct  1 20:33:45 2007
--- src/version.c	Mon Oct  1 22:50:23 2007
***************
*** 668,669 ****
--- 668,671 ----
  {   /* Add new patch number below this line */
+ /**/
+     130,
  /**/
 
-- 
FIRST SOLDIER:  So they wouldn't be able to bring a coconut back anyway.
SECOND SOLDIER: Wait a minute! Suppose two swallows carried it together?
FIRST SOLDIER:  No, they'd have to have it on a line.
                 "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD
 
 /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net   \\\
///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\        download, build and distribute -- http://www.A-A-P.org        ///
 \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///