7.1.130

   1 To: vim-dev@vim.org
   2 Subject: About patch 7.1.130
   3 Fcc: outbox
   4 From: Bram Moolenaar <Bram@moolenaar.net>
   5 Mime-Version: 1.0
   6 Content-Type: text/plain; charset=ISO-8859-1
   7 Content-Transfer-Encoding: 8bit
   8 ------------
   9
  10 Patch 7.1.130
  11 Problem:    Crash with specific order of undo and redo. (A.Politz)
  12 Solution:   Clear and adjust pointers properly.  Add u_check() for debugging.
  13 Files:      src/undo.c, src/structs.h
  14
  15
  16 *** ../vim-7.1.129/src/undo.c   Thu May 10 20:01:43 2007
  17 --- src/undo.c  Mon Oct  1 22:49:16 2007
  18 ***************
  19 *** 76,81 ****
  20 --- 76,87 ----
  21    * buffer is unloaded.
  22    */
  23
  24 + /* Uncomment the next line for including the u_check() function.  This warns
  25 +  * for errors in the debug information. */
  26 + /* #define U_DEBUG 1 */
  27 + #define UH_MAGIC 0x18dade     /* value for uh_magic when in use */
  28 + #define UE_MAGIC 0xabc123     /* value for ue_magic when in use */
  29 +
  30   #include "vim.h"
  31
  32   /* See below: use malloc()/free() for memory management. */
  33 ***************
  34 *** 113,118 ****
  35 --- 119,213 ----
  36    */
  37   static int    undo_undoes = FALSE;
  38
  39 + #ifdef U_DEBUG
  40 + /*
  41 +  * Check the undo structures for being valid.  Print a warning when something
  42 +  * looks wrong.
  43 +  */
  44 + static int seen_b_u_curhead;
  45 + static int seen_b_u_newhead;
  46 + static int header_count;
  47 +
  48 +     static void
  49 + u_check_tree(u_header_T *uhp,
  50 +       u_header_T *exp_uh_next,
  51 +       u_header_T *exp_uh_alt_prev)
  52 + {
  53 +     u_entry_T *uep;
  54 +
  55 +     if (uhp == NULL)
  56 +       return;
  57 +     ++header_count;
  58 +     if (uhp == curbuf->b_u_curhead && ++seen_b_u_curhead > 1)
  59 +     {
  60 +       EMSG("b_u_curhead found twice (looping?)");
  61 +       return;
  62 +     }
  63 +     if (uhp == curbuf->b_u_newhead && ++seen_b_u_newhead > 1)
  64 +     {
  65 +       EMSG("b_u_newhead found twice (looping?)");
  66 +       return;
  67 +     }
  68 +
  69 +     if (uhp->uh_magic != UH_MAGIC)
  70 +       EMSG("uh_magic wrong (may be using freed memory)");
  71 +     else
  72 +     {
  73 +       /* Check pointers back are correct. */
  74 +       if (uhp->uh_next != exp_uh_next)
  75 +       {
  76 +           EMSG("uh_next wrong");
  77 +           smsg((char_u *)"expected: 0x%x, actual: 0x%x",
  78 +                                                  exp_uh_next, uhp->uh_next);
  79 +       }
  80 +       if (uhp->uh_alt_prev != exp_uh_alt_prev)
  81 +       {
  82 +           EMSG("uh_alt_prev wrong");
  83 +           smsg((char_u *)"expected: 0x%x, actual: 0x%x",
  84 +                                          exp_uh_alt_prev, uhp->uh_alt_prev);
  85 +       }
  86 +
  87 +       /* Check the undo tree at this header. */
  88 +       for (uep = uhp->uh_entry; uep != NULL; uep = uep->ue_next)
  89 +       {
  90 +           if (uep->ue_magic != UE_MAGIC)
  91 +           {
  92 +               EMSG("ue_magic wrong (may be using freed memory)");
  93 +               break;
  94 +           }
  95 +       }
  96 +
  97 +       /* Check the next alt tree. */
  98 +       u_check_tree(uhp->uh_alt_next, uhp->uh_next, uhp);
  99 +
 100 +       /* Check the next header in this branch. */
 101 +       u_check_tree(uhp->uh_prev, uhp, NULL);
 102 +     }
 103 + }
 104 +
 105 +     void
 106 + u_check(int newhead_may_be_NULL)
 107 + {
 108 +     seen_b_u_newhead = 0;
 109 +     seen_b_u_curhead = 0;
 110 +     header_count = 0;
 111 +
 112 +     u_check_tree(curbuf->b_u_oldhead, NULL, NULL);
 113 +
 114 +     if (seen_b_u_newhead == 0 && curbuf->b_u_oldhead != NULL
 115 +           && !(newhead_may_be_NULL && curbuf->b_u_newhead == NULL))
 116 +       EMSGN("b_u_newhead invalid: 0x%x", curbuf->b_u_newhead);
 117 +     if (curbuf->b_u_curhead != NULL && seen_b_u_curhead == 0)
 118 +       EMSGN("b_u_curhead invalid: 0x%x", curbuf->b_u_curhead);
 119 +     if (header_count != curbuf->b_u_numhead)
 120 +     {
 121 +       EMSG("b_u_numhead invalid");
 122 +       smsg((char_u *)"expected: %ld, actual: %ld",
 123 +                              (long)header_count, (long)curbuf->b_u_numhead);
 124 +     }
 125 + }
 126 + #endif
 127 +
 128   /*
 129    * Save the current line for both the "u" and "U" command.
 130    * Returns OK or FAIL.
 131 ***************
 132 *** 243,248 ****
 133 --- 338,346 ----
 134       if (!undo_allowed())
 135         return FAIL;
 136
 137 + #ifdef U_DEBUG
 138 +     u_check(FALSE);
 139 + #endif
 140   #ifdef FEAT_NETBEANS_INTG
 141       /*
 142        * Netbeans defines areas that cannot be modified.  Bail out here when
 143 ***************
 144 *** 294,299 ****
 145 --- 392,400 ----
 146             uhp = (u_header_T *)U_ALLOC_LINE((unsigned)sizeof(u_header_T));
 147             if (uhp == NULL)
 148                 goto nomem;
 149 + #ifdef U_DEBUG
 150 +           uhp->uh_magic = UH_MAGIC;
 151 + #endif
 152         }
 153         else
 154             uhp = NULL;
 155 ***************
 156 *** 316,323 ****
 157         {
 158             u_header_T      *uhfree = curbuf->b_u_oldhead;
 159
 160 !           /* If there is no branch only free one header. */
 161 !           if (uhfree->uh_alt_next == NULL)
 162                 u_freeheader(curbuf, uhfree, &old_curhead);
 163             else
 164             {
 165 --- 417,427 ----
 166         {
 167             u_header_T      *uhfree = curbuf->b_u_oldhead;
 168
 169 !           if (uhfree == old_curhead)
 170 !               /* Can't reconnect the branch, delete all of it. */
 171 !               u_freebranch(curbuf, uhfree, &old_curhead);
 172 !           else if (uhfree->uh_alt_next == NULL)
 173 !               /* There is no branch, only free one header. */
 174                 u_freeheader(curbuf, uhfree, &old_curhead);
 175             else
 176             {
 177 ***************
 178 *** 326,331 ****
 179 --- 430,438 ----
 180                     uhfree = uhfree->uh_alt_next;
 181                 u_freebranch(curbuf, uhfree, &old_curhead);
 182             }
 183 + #ifdef U_DEBUG
 184 +           u_check(TRUE);
 185 + #endif
 186         }
 187
 188         if (uhp == NULL)                /* no undo at all */
 189 ***************
 190 *** 478,483 ****
 191 --- 585,593 ----
 192       uep = (u_entry_T *)U_ALLOC_LINE((unsigned)sizeof(u_entry_T));
 193       if (uep == NULL)
 194         goto nomem;
 195 + #ifdef U_DEBUG
 196 +     uep->ue_magic = UE_MAGIC;
 197 + #endif
 198
 199       uep->ue_size = size;
 200       uep->ue_top = top;
 201 ***************
 202 *** 525,530 ****
 203 --- 635,643 ----
 204       curbuf->b_u_synced = FALSE;
 205       undo_undoes = FALSE;
 206
 207 + #ifdef U_DEBUG
 208 +     u_check(FALSE);
 209 + #endif
 210       return OK;
 211
 212   nomem:
 213 ***************
 214 *** 955,960 ****
 215 --- 1068,1076 ----
 216       int               empty_buffer;               /* buffer became empty */
 217       u_header_T        *curhead = curbuf->b_u_curhead;
 218
 219 + #ifdef U_DEBUG
 220 +     u_check(FALSE);
 221 + #endif
 222       old_flags = curhead->uh_flags;
 223       new_flags = (curbuf->b_changed ? UH_CHANGED : 0) +
 224                ((curbuf->b_ml.ml_flags & ML_EMPTY) ? UH_EMPTYBUF : 0);
 225 ***************
 226 *** 1186,1191 ****
 227 --- 1302,1310 ----
 228       /* The timestamp can be the same for multiple changes, just use the one of
 229        * the undone/redone change. */
 230       curbuf->b_u_seq_time = curhead->uh_time;
 231 + #ifdef U_DEBUG
 232 +     u_check(FALSE);
 233 + #endif
 234   }
 235
 236   /*
 237 ***************
 238 *** 1515,1521 ****
 239   }
 240
 241   /*
 242 !  * Free one header and its entry list and adjust the pointers.
 243    */
 244       static void
 245   u_freeheader(buf, uhp, uhpp)
 246 --- 1634,1640 ----
 247   }
 248
 249   /*
 250 !  * Free one header "uhp" and its entry list and adjust the pointers.
 251    */
 252       static void
 253   u_freeheader(buf, uhp, uhpp)
 254 ***************
 255 *** 1523,1528 ****
 256 --- 1642,1649 ----
 257       u_header_T            *uhp;
 258       u_header_T            **uhpp;     /* if not NULL reset when freeing this header */
 259   {
 260 +     u_header_T            *uhap;
 261 +
 262       /* When there is an alternate redo list free that branch completely,
 263        * because we can never go there. */
 264       if (uhp->uh_alt_next != NULL)
 265 ***************
 266 *** 1540,1546 ****
 267       if (uhp->uh_prev == NULL)
 268         buf->b_u_newhead = uhp->uh_next;
 269       else
 270 !       uhp->uh_prev->uh_next = uhp->uh_next;
 271
 272       u_freeentries(buf, uhp, uhpp);
 273   }
 274 --- 1661,1668 ----
 275       if (uhp->uh_prev == NULL)
 276         buf->b_u_newhead = uhp->uh_next;
 277       else
 278 !       for (uhap = uhp->uh_prev; uhap != NULL; uhap = uhap->uh_alt_next)
 279 !           uhap->uh_next = uhp->uh_next;
 280
 281       u_freeentries(buf, uhp, uhpp);
 282   }
 283 ***************
 284 *** 1585,1590 ****
 285 --- 1707,1714 ----
 286       /* Check for pointers to the header that become invalid now. */
 287       if (buf->b_u_curhead == uhp)
 288         buf->b_u_curhead = NULL;
 289 +     if (buf->b_u_newhead == uhp)
 290 +       buf->b_u_newhead = NULL;  /* freeing the newest entry */
 291       if (uhpp != NULL && uhp == *uhpp)
 292         *uhpp = NULL;
 293
 294 ***************
 295 *** 1594,1599 ****
 296 --- 1718,1726 ----
 297         u_freeentry(uep, uep->ue_size);
 298       }
 299
 300 + #ifdef U_DEBUG
 301 +     uhp->uh_magic = 0;
 302 + #endif
 303       U_FREE_LINE((char_u *)uhp);
 304       --buf->b_u_numhead;
 305   }
 306 ***************
 307 *** 1609,1614 ****
 308 --- 1736,1744 ----
 309       while (n > 0)
 310         U_FREE_LINE(uep->ue_array[--n]);
 311       U_FREE_LINE((char_u *)uep->ue_array);
 312 + #ifdef U_DEBUG
 313 +     uep->ue_magic = 0;
 314 + #endif
 315       U_FREE_LINE((char_u *)uep);
 316   }
 317
 318 *** ../vim-7.1.129/src/structs.h        Sun Aug 12 15:50:26 2007
 319 --- src/structs.h       Sat Sep 29 15:03:38 2007
 320 ***************
 321 *** 278,283 ****
 322 --- 278,286 ----
 323       linenr_T  ue_lcount;      /* linecount when u_save called */
 324       char_u    **ue_array;     /* array of lines in undo block */
 325       long      ue_size;        /* number of lines in ue_array */
 326 + #ifdef U_DEBUG
 327 +     int               ue_magic;       /* magic number to check allocation */
 328 + #endif
 329   };
 330
 331   struct u_header
 332 ***************
 333 *** 300,305 ****
 334 --- 303,311 ----
 335       visualinfo_T uh_visual;   /* Visual areas before undo/after redo */
 336   #endif
 337       time_t    uh_time;        /* timestamp when the change was made */
 338 + #ifdef U_DEBUG
 339 +     int               uh_magic;       /* magic number to check allocation */
 340 + #endif
 341   };
 342
 343   /* values for uh_flags */
 344 *** ../vim-7.1.129/src/version.c        Mon Oct  1 20:33:45 2007
 345 --- src/version.c       Mon Oct  1 22:50:23 2007
 346 ***************
 347 *** 668,669 ****
 348 --- 668,671 ----
 349   {   /* Add new patch number below this line */
 350 + /**/
 351 +     130,
 352   /**/
 353
 354 --
 355 FIRST SOLDIER:  So they wouldn't be able to bring a coconut back anyway.
 356 SECOND SOLDIER: Wait a minute! Suppose two swallows carried it together?
 357 FIRST SOLDIER:  No, they'd have to have it on a line.
 358                  "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD
 359
 360  /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net   \\\
 361 ///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
 362 \\\        download, build and distribute -- http://www.A-A-P.org        ///
 363  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///