[Emacs-diffs] /srv/bzr/emacs/trunk r105407: Fix bug #9221 with memory leak in bidi display.

[Eli Zaretskii]

------------------------------------------------------------
revno: 105407 [merge]
fixes bug(s): http://debbugs.gnu.org/9221
committer: Eli Zaretskii <address@hidden>
branch nick: trunk
timestamp: Fri 2011-08-05 14:04:44 +0300
message:
  Fix bug #9221 with memory leak in bidi display.
  Add code to monitor memory allocation for bidi cache shelving.
  
   src/xdisp.c (display_line): Release buffer allocated for shelved bidi
   cache.
   src/bidi.c (bidi_shelve_cache, bidi_unshelve_cache): Track total
   amount allocated this far in `bidi_cache_total_alloc'.
   (bidi_unshelve_cache): Accept an additional argument JUST_FREE; if
   non-zero, only free the data buffer without restoring the cache
   contents.  All callers changed.
   src/dispextern.h (bidi_unshelve_cache): Update prototype.
   src/xdisp.c (SAVE_IT, pos_visible_p, move_it_in_display_line_to)
   (move_it_in_display_line, move_it_to)
   (move_it_vertically_backward, move_it_by_lines): Replace the call
   to xfree to an equivalent call to bidi_unshelve_cache.
   (move_it_in_display_line_to): Fix logic of returning
   MOVE_POS_MATCH_OR_ZV in the bidi case.
modified:
  src/ChangeLog
  src/bidi.c
  src/dispextern.h
  src/dispnew.c
  src/indent.c
  src/window.c
  src/xdisp.c

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2011-08-05 10:32:06 +0000
+++ b/src/ChangeLog     2011-08-05 11:04:44 +0000
@@ -1,5 +1,25 @@
 2011-08-05  Eli Zaretskii  <address@hidden>
 
+       *xdisp.c (display_line): Release buffer allocated for shelved bidi
+       cache.  (Bug#9221)
+
+       * bidi.c (bidi_shelve_cache, bidi_unshelve_cache): Track total
+       amount allocated this far in `bidi_cache_total_alloc'.
+       (bidi_unshelve_cache): Accept an additional argument JUST_FREE; if
+       non-zero, only free the data buffer without restoring the cache
+       contents.  All callers changed.
+
+       * dispextern.h (bidi_unshelve_cache): Update prototype.
+
+       * xdisp.c (SAVE_IT, pos_visible_p, move_it_in_display_line_to)
+       (move_it_in_display_line, move_it_to)
+       (move_it_vertically_backward, move_it_by_lines): Replace the call
+       to xfree to an equivalent call to bidi_unshelve_cache.
+       (move_it_in_display_line_to): Fix logic of returning
+       MOVE_POS_MATCH_OR_ZV in the bidi case.
+
+2011-08-05  Eli Zaretskii  <address@hidden>
+
        * xdisp.c (set_cursor_from_row): Prefer the candidate glyph that
        came from a string character with a `cursor' property.  (Bug#9229)
 

=== modified file 'src/bidi.c'
--- a/src/bidi.c        2011-08-02 19:16:32 +0000
+++ b/src/bidi.c        2011-08-05 11:04:44 +0000
@@ -620,12 +620,15 @@
   bidi_cache_last_idx = -1;
 }
 
+ptrdiff_t bidi_cache_total_alloc;
+
 /* Stash away a copy of the cache and its control variables.  */
 void *
 bidi_shelve_cache (void)
 {
   unsigned char *databuf;
 
+  /* Empty cache.  */
   if (bidi_cache_idx == 0)
     return NULL;
 
@@ -634,6 +637,12 @@
                     + sizeof (bidi_cache_start_stack)
                     + sizeof (bidi_cache_sp) + sizeof (bidi_cache_start)
                     + sizeof (bidi_cache_last_idx));
+  bidi_cache_total_alloc +=
+    sizeof (bidi_cache_idx) + bidi_cache_idx * sizeof (struct bidi_it)
+    + sizeof (bidi_cache_start_stack)
+    + sizeof (bidi_cache_sp) + sizeof (bidi_cache_start)
+    + sizeof (bidi_cache_last_idx);
+
   memcpy (databuf, &bidi_cache_idx, sizeof (bidi_cache_idx));
   memcpy (databuf + sizeof (bidi_cache_idx),
          bidi_cache, bidi_cache_idx * sizeof (struct bidi_it));
@@ -659,7 +668,7 @@
 
 /* Restore the cache state from a copy stashed away by bidi_shelve_cache.  */
 void
-bidi_unshelve_cache (void *databuf)
+bidi_unshelve_cache (void *databuf, int just_free)
 {
   unsigned char *p = databuf;
 
@@ -672,30 +681,47 @@
     }
   else
     {
-      memcpy (&bidi_cache_idx, p, sizeof (bidi_cache_idx));
-      bidi_cache_ensure_space (bidi_cache_idx);
-      memcpy (bidi_cache, p + sizeof (bidi_cache_idx),
-             bidi_cache_idx * sizeof (struct bidi_it));
-      memcpy (bidi_cache_start_stack,
-             p + sizeof (bidi_cache_idx)
-             + bidi_cache_idx * sizeof (struct bidi_it),
-             sizeof (bidi_cache_start_stack));
-      memcpy (&bidi_cache_sp,
-             p + sizeof (bidi_cache_idx)
-             + bidi_cache_idx * sizeof (struct bidi_it)
-             + sizeof (bidi_cache_start_stack),
-             sizeof (bidi_cache_sp));
-      memcpy (&bidi_cache_start,
-             p + sizeof (bidi_cache_idx)
-             + bidi_cache_idx * sizeof (struct bidi_it)
-             + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp),
-             sizeof (bidi_cache_start));
-      memcpy (&bidi_cache_last_idx,
-             p + sizeof (bidi_cache_idx)
-             + bidi_cache_idx * sizeof (struct bidi_it)
-             + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
-             + sizeof (bidi_cache_start),
-             sizeof (bidi_cache_last_idx));
+      if (just_free)
+       {
+         ptrdiff_t idx;
+
+         memcpy (&idx, p, sizeof (bidi_cache_idx));
+         bidi_cache_total_alloc -=
+           sizeof (bidi_cache_idx) + idx * sizeof (struct bidi_it)
+           + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
+           + sizeof (bidi_cache_start) + sizeof (bidi_cache_last_idx);
+       }
+      else
+       {
+         memcpy (&bidi_cache_idx, p, sizeof (bidi_cache_idx));
+         bidi_cache_ensure_space (bidi_cache_idx);
+         memcpy (bidi_cache, p + sizeof (bidi_cache_idx),
+                 bidi_cache_idx * sizeof (struct bidi_it));
+         memcpy (bidi_cache_start_stack,
+                 p + sizeof (bidi_cache_idx)
+                 + bidi_cache_idx * sizeof (struct bidi_it),
+                 sizeof (bidi_cache_start_stack));
+         memcpy (&bidi_cache_sp,
+                 p + sizeof (bidi_cache_idx)
+                 + bidi_cache_idx * sizeof (struct bidi_it)
+                 + sizeof (bidi_cache_start_stack),
+                 sizeof (bidi_cache_sp));
+         memcpy (&bidi_cache_start,
+                 p + sizeof (bidi_cache_idx)
+                 + bidi_cache_idx * sizeof (struct bidi_it)
+                 + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp),
+                 sizeof (bidi_cache_start));
+         memcpy (&bidi_cache_last_idx,
+                 p + sizeof (bidi_cache_idx)
+                 + bidi_cache_idx * sizeof (struct bidi_it)
+                 + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
+                 + sizeof (bidi_cache_start),
+                 sizeof (bidi_cache_last_idx));
+         bidi_cache_total_alloc -=
+           sizeof (bidi_cache_idx) + bidi_cache_idx * sizeof (struct bidi_it)
+           + sizeof (bidi_cache_start_stack) + sizeof (bidi_cache_sp)
+           + sizeof (bidi_cache_start) + sizeof (bidi_cache_last_idx);
+       }
 
       xfree (p);
     }

=== modified file 'src/dispextern.h'
--- a/src/dispextern.h  2011-08-02 19:16:32 +0000
+++ b/src/dispextern.h  2011-08-05 11:04:44 +0000
@@ -2978,7 +2978,7 @@
 extern void bidi_push_it (struct bidi_it *);
 extern void bidi_pop_it (struct bidi_it *);
 extern void *bidi_shelve_cache (void);
-extern void bidi_unshelve_cache (void *);
+extern void bidi_unshelve_cache (void *, int);
 
 /* Defined in xdisp.c */
 

=== modified file 'src/dispnew.c'
--- a/src/dispnew.c     2011-07-14 20:40:35 +0000
+++ b/src/dispnew.c     2011-08-05 11:04:44 +0000
@@ -5282,7 +5282,7 @@
      argument is ZV to prevent move_it_in_display_line from matching
      based on buffer positions.  */
   move_it_in_display_line (&it, ZV, to_x, MOVE_TO_X);
-  bidi_unshelve_cache (itdata);
+  bidi_unshelve_cache (itdata, 0);
 
   Fset_buffer (old_current_buffer);
 

=== modified file 'src/indent.c'
--- a/src/indent.c      2011-07-14 21:35:23 +0000
+++ b/src/indent.c      2011-08-05 11:04:44 +0000
@@ -2135,7 +2135,7 @@
        }
 
       SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
-      bidi_unshelve_cache (itdata);
+      bidi_unshelve_cache (itdata, 0);
     }
 
   if (BUFFERP (old_buffer))

=== modified file 'src/window.c'
--- a/src/window.c      2011-07-14 17:28:42 +0000
+++ b/src/window.c      2011-08-05 11:04:44 +0000
@@ -1379,7 +1379,7 @@
       if (it.current_y < it.last_visible_y)
        move_it_past_eol (&it);
       value = make_number (IT_CHARPOS (it));
-      bidi_unshelve_cache (itdata);
+      bidi_unshelve_cache (itdata, 0);
 
       if (old_buffer)
        set_buffer_internal (old_buffer);
@@ -4273,7 +4273,7 @@
        }
 
       start = it.current.pos;
-      bidi_unshelve_cache (itdata);
+      bidi_unshelve_cache (itdata, 0);
     }
   else if (auto_window_vscroll_p)
     {
@@ -4417,7 +4417,7 @@
            }
          else
            {
-             bidi_unshelve_cache (itdata);
+             bidi_unshelve_cache (itdata, 0);
              if (noerror)
                return;
              else if (n < 0)   /* could happen with empty buffers */
@@ -4434,7 +4434,7 @@
            w->vscroll = 0;
          else
            {
-             bidi_unshelve_cache (itdata);
+             bidi_unshelve_cache (itdata, 0);
              if (noerror)
                return;
              else
@@ -4583,7 +4583,7 @@
            SET_PT_BOTH (charpos, bytepos);
        }
     }
-  bidi_unshelve_cache (itdata);
+  bidi_unshelve_cache (itdata, 0);
 }
 
 
@@ -5010,7 +5010,7 @@
   start_display (&it, w, start);
   move_it_vertically (&it, height);
   bottom_y = line_bottom_y (&it);
-  bidi_unshelve_cache (itdata);
+  bidi_unshelve_cache (itdata, 0);
 
   /* rms: On a non-window display,
      the value of it.vpos at the bottom of the screen
@@ -5116,7 +5116,7 @@
          move_it_vertically_backward (&it, window_box_height (w) / 2);
          charpos = IT_CHARPOS (it);
          bytepos = IT_BYTEPOS (it);
-         bidi_unshelve_cache (itdata);
+         bidi_unshelve_cache (itdata, 0);
        }
       else if (iarg < 0)
        {
@@ -5164,7 +5164,7 @@
            }
          if (h <= 0)
            {
-             bidi_unshelve_cache (itdata);
+             bidi_unshelve_cache (itdata, 0);
              return Qnil;
            }
 
@@ -5187,7 +5187,7 @@
          charpos = IT_CHARPOS (it);
          bytepos = IT_BYTEPOS (it);
 
-         bidi_unshelve_cache (itdata);
+         bidi_unshelve_cache (itdata, 0);
        }
       else
        {

=== modified file 'src/xdisp.c'
--- a/src/xdisp.c       2011-08-05 10:32:06 +0000
+++ b/src/xdisp.c       2011-08-05 11:04:44 +0000
@@ -604,7 +604,7 @@
 #define SAVE_IT(ITCOPY,ITORIG,CACHE)           \
   do {                                         \
     if (CACHE)                                 \
-      xfree (CACHE);                           \
+      bidi_unshelve_cache (CACHE, 1);          \
     ITCOPY = ITORIG;                           \
     CACHE = bidi_shelve_cache();               \
   } while (0)
@@ -613,7 +613,7 @@
   do {                                         \
     if (pITORIG != pITCOPY)                    \
       *(pITORIG) = *(pITCOPY);                 \
-    bidi_unshelve_cache (CACHE);               \
+    bidi_unshelve_cache (CACHE, 0);            \
     CACHE = NULL;                              \
   } while (0)
 
@@ -1341,9 +1341,9 @@
          *vpos = it2.vpos;
        }
       else
-       xfree (it2data);
+       bidi_unshelve_cache (it2data, 1);
     }
-  bidi_unshelve_cache (itdata);
+  bidi_unshelve_cache (itdata, 0);
 
   if (old_buffer)
     set_buffer_internal_1 (old_buffer);
@@ -2627,7 +2627,7 @@
            it->paragraph_embedding = R2L;
          else
            it->paragraph_embedding = NEUTRAL_DIR;
-         bidi_unshelve_cache (NULL);
+         bidi_unshelve_cache (NULL, 0);
          bidi_init_it (charpos, IT_BYTEPOS (*it), FRAME_WINDOW_P (it->f),
                        &it->bidi_it);
        }
@@ -5618,7 +5618,7 @@
        pos = --IT_CHARPOS (it2);
        --IT_BYTEPOS (it2);
        it2.sp = 0;
-       bidi_unshelve_cache (NULL);
+       bidi_unshelve_cache (NULL, 0);
        it2.string_from_display_prop_p = 0;
        it2.from_disp_prop_p = 0;
        if (handle_display_prop (&it2) == HANDLED_RETURN
@@ -5828,7 +5828,7 @@
     {
       bidi_init_it (IT_CHARPOS (*it), IT_BYTEPOS (*it), FRAME_WINDOW_P (it->f),
                    &it->bidi_it);
-      bidi_unshelve_cache (NULL);
+      bidi_unshelve_cache (NULL, 0);
       it->bidi_it.paragraph_dir = NEUTRAL_DIR;
       it->bidi_it.string.s = NULL;
       it->bidi_it.string.lstring = Qnil;
@@ -8009,13 +8009,13 @@
             positions smaller than TO_CHARPOS, return
             MOVE_POS_MATCH_OR_ZV, like the unidirectional display
             did.  */
-         if ((op & MOVE_TO_POS) != 0
+         if (it->bidi_p && (op & MOVE_TO_POS) != 0
              && !saw_smaller_pos
              && IT_CHARPOS (*it) > to_charpos)
            {
-             result = MOVE_POS_MATCH_OR_ZV;
-             if (it->bidi_p && IT_CHARPOS (ppos_it) < ZV)
+             if (IT_CHARPOS (ppos_it) < ZV)
                RESTORE_IT (it, &ppos_it, ppos_data);
+             goto buffer_pos_reached;
            }
          else
            result = MOVE_NEWLINE_OR_CR;
@@ -8054,14 +8054,13 @@
                     character positions smaller than TO_CHARPOS,
                     return MOVE_POS_MATCH_OR_ZV, like the
                     unidirectional display did.  */
-                 || ((op & MOVE_TO_POS) != 0
+                 || (it->bidi_p && (op & MOVE_TO_POS) != 0
                      && !saw_smaller_pos
                      && IT_CHARPOS (*it) > to_charpos))
                {
-                 result = MOVE_POS_MATCH_OR_ZV;
-                 if (it->bidi_p && !at_eob_p && IT_CHARPOS (ppos_it) < ZV)
+                 if (!at_eob_p && IT_CHARPOS (ppos_it) < ZV)
                    RESTORE_IT (it, &ppos_it, ppos_data);
-                 break;
+                 goto buffer_pos_reached;
                }
              if (ITERATOR_AT_END_OF_LINE_P (it))
                {
@@ -8069,14 +8068,13 @@
                  break;
                }
            }
-         else if ((op & MOVE_TO_POS) != 0
+         else if (it->bidi_p && (op & MOVE_TO_POS) != 0
                   && !saw_smaller_pos
                   && IT_CHARPOS (*it) > to_charpos)
            {
-             result = MOVE_POS_MATCH_OR_ZV;
-             if (it->bidi_p && IT_CHARPOS (ppos_it) < ZV)
+             if (IT_CHARPOS (ppos_it) < ZV)
                RESTORE_IT (it, &ppos_it, ppos_data);
-             break;
+             goto buffer_pos_reached;
            }
          result = MOVE_LINE_TRUNCATED;
          break;
@@ -8096,13 +8094,13 @@
  done:
 
   if (atpos_data)
-    xfree (atpos_data);
+    bidi_unshelve_cache (atpos_data, 1);
   if (atx_data)
-    xfree (atx_data);
+    bidi_unshelve_cache (atx_data, 1);
   if (wrap_data)
-    xfree (wrap_data);
+    bidi_unshelve_cache (wrap_data, 1);
   if (ppos_data)
-    xfree (ppos_data);
+    bidi_unshelve_cache (ppos_data, 1);
 
   /* Restore the iterator settings altered at the beginning of this
      function.  */
@@ -8137,7 +8135,7 @@
            (it, -1, prev_x, MOVE_TO_X);
        }
       else
-       xfree (save_data);
+       bidi_unshelve_cache (save_data, 1);
     }
   else
     move_it_in_display_line_to (it, to_charpos, to_x, op);
@@ -8396,7 +8394,7 @@
     }
 
   if (backup_data)
-    xfree (backup_data);
+    bidi_unshelve_cache (backup_data, 1);
 
   TRACE_MOVE ((stderr, "move_it_to: reached %d\n", reached));
 }
@@ -8475,7 +8473,7 @@
       RESTORE_IT (it, it, it2data);
       if (nlines > 0)
        move_it_by_lines (it, nlines);
-      xfree (it3data);
+      bidi_unshelve_cache (it3data, 1);
     }
   else
     {
@@ -8671,7 +8669,7 @@
          if (IT_CHARPOS (*it) >= start_charpos)
            RESTORE_IT (it, &it2, it2data);
          else
-           xfree (it2data);
+           bidi_unshelve_cache (it2data, 1);
        }
       else
        RESTORE_IT (it, it, it2data);
@@ -18779,6 +18777,9 @@
        }
     }
 
+  if (wrap_data)
+    bidi_unshelve_cache (wrap_data, 1);
+
   /* If line is not empty and hscrolled, maybe insert truncation glyphs
      at the left window margin.  */
   if (it->first_visible_x