To: vim-dev@vim.org Subject: About patch 7.1.130 Fcc: outbox From: Bram Moolenaar 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 ///