[PATCH] fix crash in glyph-cachels
Ben Wing
ben at xemacs.org
Wed Feb 3 22:25:16 EST 2010
changeset: 4954:4d35e52790f8
tag: tip
user: Ben Wing <ben at xemacs.org>
date: Wed Feb 03 21:06:14 2010 -0600
files: src/ChangeLog src/frame.c src/glyphs.c src/redisplay.c
description:
fix crash in glyph-cachels
-------------------- ChangeLog entries follow: --------------------
src/ChangeLog addition:
2010-02-03 Ben Wing <ben at xemacs.org>
* frame.c (Fmake_frame):
* glyphs.c:
* glyphs.c (NUM_PRECACHED_GLYPHS):
* glyphs.c (get_glyph_cachel_index):
* glyphs.c (FROB):
* glyphs.c (mark_glyph_cachels_as_not_updated):
* redisplay.c (regenerate_window):
* redisplay.c (redisplay_window):
When creating a frame, call reset_glyph_cachels on the minibuffer
window as well as the root window. Fixes a crash due to other
glyphs (e.g. the gutter glyph) getting in the glyph cachel before
the pre-cached glyphs that are supposed to have fixed indices
(continuation-glyph, truncation-glyph, etc.).
Add a bunch of asserts to make sure that the glyph cachels always
properly contain the pre-cached glyphs.
diff -r 0d4c9d0f6a8d -r 4d35e52790f8 src/ChangeLog
--- a/src/ChangeLog Wed Feb 03 20:51:18 2010 -0600
+++ b/src/ChangeLog Wed Feb 03 21:06:14 2010 -0600
@@ -1,3 +1,22 @@
+2010-02-03 Ben Wing <ben at xemacs.org>
+
+ * frame.c (Fmake_frame):
+ * glyphs.c:
+ * glyphs.c (NUM_PRECACHED_GLYPHS):
+ * glyphs.c (get_glyph_cachel_index):
+ * glyphs.c (FROB):
+ * glyphs.c (mark_glyph_cachels_as_not_updated):
+ * redisplay.c (regenerate_window):
+ * redisplay.c (redisplay_window):
+ When creating a frame, call reset_glyph_cachels on the minibuffer
+ window as well as the root window. Fixes a crash due to other
+ glyphs (e.g. the gutter glyph) getting in the glyph cachel before
+ the pre-cached glyphs that are supposed to have fixed indices
+ (continuation-glyph, truncation-glyph, etc.).
+
+ Add a bunch of asserts to make sure that the glyph cachels always
+ properly contain the pre-cached glyphs.
+
2010-02-03 Ben Wing <ben at xemacs.org>
* device-x.c (x_get_resource_prefix):
diff -r 0d4c9d0f6a8d -r 4d35e52790f8 src/frame.c
--- a/src/frame.c Wed Feb 03 20:51:18 2010 -0600
+++ b/src/frame.c Wed Feb 03 21:06:14 2010 -0600
@@ -585,6 +585,8 @@
update_frame_window_mirror (f);
+ /* #### Do we need to be calling reset_face_cachels here, and then again
+ down below? */
if (initialized && !DEVICE_STREAM_P (d))
{
if (!NILP (f->minibuffer_window))
@@ -642,8 +644,19 @@
things. */
init_frame_toolbars (f);
#endif
+ /* Added this assert recently (2-1-10); seems there should be only
+ two windows, root and minibufer. Probably we should just be
+ calling reset_*_cachels on the root window directly instead of the
+ selected window, but I want to make sure they are always the
+ same. --ben */
+ assert (EQ (FRAME_SELECTED_WINDOW (f), f->root_window));
reset_face_cachels (XWINDOW (FRAME_SELECTED_WINDOW (f)));
reset_glyph_cachels (XWINDOW (FRAME_SELECTED_WINDOW (f)));
+ if (!NILP (f->minibuffer_window))
+ {
+ reset_face_cachels (XWINDOW (f->minibuffer_window));
+ reset_glyph_cachels (XWINDOW (f->minibuffer_window));
+ }
change_frame_size (f, f->height, f->width, 0);
}
diff -r 0d4c9d0f6a8d -r 4d35e52790f8 src/glyphs.c
--- a/src/glyphs.c Wed Feb 03 20:51:18 2010 -0600
+++ b/src/glyphs.c Wed Feb 03 21:06:14 2010 -0600
@@ -4276,8 +4276,18 @@
/*****************************************************************************
- * glyph cachel functions *
- *****************************************************************************/
+ * glyph cachel functions *
+ *****************************************************************************/
+
+#define NUM_PRECACHED_GLYPHS 6
+#define LOOP_OVER_PRECACHED_GLYPHS \
+ FROB (Vcontinuation_glyph, CONT_GLYPH_INDEX) \
+ FROB (Vtruncation_glyph, TRUN_GLYPH_INDEX) \
+ FROB (Vhscroll_glyph, HSCROLL_GLYPH_INDEX) \
+ FROB (Vcontrol_arrow_glyph, CONTROL_GLYPH_INDEX) \
+ FROB (Voctal_escape_glyph, OCT_ESC_GLYPH_INDEX) \
+ FROB (Vinvisible_text_glyph, INVIS_GLYPH_INDEX)
+
/* #### All of this is 95% copied from face cachels. Consider
consolidating.
@@ -4352,6 +4362,27 @@
Dynarr_add (w->glyph_cachels, new_cachel);
}
+#ifdef ERROR_CHECK_GLYPHS
+
+/* The precached glyphs should always occur in slots 0 - 5, with each glyph in the
+ slot reserved for it. Meanwhile any other glyphs should always occur in slots
+ 6 or greater. */
+static void
+verify_glyph_index (Lisp_Object glyph, glyph_index idx)
+{
+ if (0)
+ ;
+#define FROB(glyph_obj, gindex) \
+ else if (EQ (glyph, glyph_obj)) \
+ assert (gindex == idx);
+ LOOP_OVER_PRECACHED_GLYPHS
+ else
+ assert (idx >= NUM_PRECACHED_GLYPHS);
+#undef FROB
+}
+
+#endif /* ERROR_CHECK_GLYPHS */
+
glyph_index
get_glyph_cachel_index (struct window *w, Lisp_Object glyph)
{
@@ -4367,6 +4398,9 @@
if (EQ (cachel->glyph, glyph) && !NILP (glyph))
{
+#ifdef ERROR_CHECK_GLYPHS
+ verify_glyph_index (glyph, elt);
+#endif /* ERROR_CHECK_GLYPHS */
update_glyph_cachel_data (w, glyph, cachel);
return elt;
}
@@ -4381,12 +4415,10 @@
reset_glyph_cachels (struct window *w)
{
Dynarr_reset (w->glyph_cachels);
- get_glyph_cachel_index (w, Vcontinuation_glyph);
- get_glyph_cachel_index (w, Vtruncation_glyph);
- get_glyph_cachel_index (w, Vhscroll_glyph);
- get_glyph_cachel_index (w, Vcontrol_arrow_glyph);
- get_glyph_cachel_index (w, Voctal_escape_glyph);
- get_glyph_cachel_index (w, Vinvisible_text_glyph);
+#define FROB(glyph_obj, gindex) \
+ get_glyph_cachel_index (w, glyph_obj);
+ LOOP_OVER_PRECACHED_GLYPHS
+#undef FROB
}
void
@@ -4394,19 +4426,18 @@
{
int elt;
+ /* A previous bug resulted from the glyph cachels never getting reset
+ in the minibuffer window after creation, and another glyph added before
+ we got a chance to add the six normal glyphs that should go first, and
+ we got called with only one glyph present. */
+ assert (Dynarr_length (w->glyph_cachels) >= NUM_PRECACHED_GLYPHS);
/* We need to have a dirty flag to tell if the glyph has changed.
We can check to see if each glyph variable is actually a
completely different glyph, though. */
#define FROB(glyph_obj, gindex) \
update_glyph_cachel_data (w, glyph_obj, \
- Dynarr_atp (w->glyph_cachels, gindex))
-
- FROB (Vcontinuation_glyph, CONT_GLYPH_INDEX);
- FROB (Vtruncation_glyph, TRUN_GLYPH_INDEX);
- FROB (Vhscroll_glyph, HSCROLL_GLYPH_INDEX);
- FROB (Vcontrol_arrow_glyph, CONTROL_GLYPH_INDEX);
- FROB (Voctal_escape_glyph, OCT_ESC_GLYPH_INDEX);
- FROB (Vinvisible_text_glyph, INVIS_GLYPH_INDEX);
+ Dynarr_atp (w->glyph_cachels, gindex));
+ LOOP_OVER_PRECACHED_GLYPHS
#undef FROB
for (elt = 0; elt < Dynarr_length (w->glyph_cachels); elt++)
@@ -4449,7 +4480,7 @@
/*****************************************************************************
- * subwindow cachel functions *
+ * subwindow cachel functions *
*****************************************************************************/
/* Subwindows are curious in that you have to physically unmap them to
not display them. It is problematic deciding what to do in
@@ -4546,7 +4577,7 @@
}
/*****************************************************************************
- * subwindow exposure ignorance *
+ * subwindow exposure ignorance *
*****************************************************************************/
/* when we unmap subwindows the associated window system will generate
expose events. This we do not want as redisplay already copes with
diff -r 0d4c9d0f6a8d -r 4d35e52790f8 src/redisplay.c
--- a/src/redisplay.c Wed Feb 03 20:51:18 2010 -0600
+++ b/src/redisplay.c Wed Feb 03 21:06:14 2010 -0600
@@ -5471,6 +5471,17 @@
Dynarr_reset (dla);
w->max_line_len = 0;
+ /* Added 2-1-10 -- we should never have empty face or glyph cachels
+ because we initialized them at startup and the only way to reduce
+ their number is through calling reset_face_cachels() or
+ reset_glyph_cachels(), which as a side effect sets up a number of
+ standard entries */
+ assert (Dynarr_length (w->face_cachels));
+ assert (Dynarr_length (w->glyph_cachels));
+
+#if 0
+ /* #### Delete this code sometime later than 2-1-10 when we're sure it's
+ not needed */
/* Normally these get updated in redisplay_window but it is possible
for this function to get called from some other points where that
update may not have occurred. This acts as a safety check. */
@@ -5478,6 +5489,7 @@
reset_face_cachels (w);
if (!Dynarr_length (w->glyph_cachels))
reset_glyph_cachels (w);
+#endif
Fset_marker (w->start[type], make_int (start_pos), w->buffer);
Fset_marker (w->pointm[type], make_int (point), w->buffer);
@@ -6288,10 +6300,22 @@
}
Fset_marker (w->pointm[DESIRED_DISP], make_int (pointm), the_buffer);
+ /* Added 2-1-10 -- we should never have empty face or glyph cachels
+ because we initialized them at startup and the only way to reduce
+ their number is through calling reset_face_cachels() or
+ reset_glyph_cachels(), which as a side effect sets up a number of
+ standard entries */
+ assert (Dynarr_length (w->face_cachels));
+ assert (Dynarr_length (w->glyph_cachels));
+
/* If the buffer has changed we have to invalidate all of our face
cache elements. */
if ((!echo_active && b != window_display_buffer (w))
+#if 0
+ /* #### Delete this code sometime later than 2-1-10 when we're sure it's
+ not needed */
|| !Dynarr_length (w->face_cachels)
+#endif
|| f->faces_changed)
reset_face_cachels (w);
else
@@ -6301,7 +6325,12 @@
the cache purely because glyphs have changed - this is now
handled by the dirty flag.*/
if ((!echo_active && b != window_display_buffer (w))
- || !Dynarr_length (w->glyph_cachels) || f->faces_changed)
+#if 0
+ /* #### Delete this code sometime later than 2-1-10 when we're sure it's
+ not needed */
+ || !Dynarr_length (w->glyph_cachels)
+#endif
+ || f->faces_changed)
reset_glyph_cachels (w);
else
mark_glyph_cachels_as_not_updated (w);
More information about the XEmacs-Patches
mailing list