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    ///