[PATCH] Prevent a crash if the ASCII charset registries match nothing

Ar an deichiú lá de mí na Samhain, scríobh stephen(a)xemacs.org: Eh, still, XEmacs shouldn’t be crashable from Lisp. The below has a bit more testing, with X11 and GTK; I intend committing it this evening. SUPERSEDES 17746.1739.273694.339710(a)parhasard.net src/ChangeLog addition: 2006-11-11 Aidan Kehoe <kehoea(a)parhasard.net&gt; * charset.h: * mule-charset.c (set_charset_registries): Provide a C-accessible version of set-charset-registries that doesn't error. Called from x_find_charset_font. * faces.c (ensure_face_cachel_contains_charset): Correct my spelling. * faces.c (update_EmacsFrame): Don't update the frame if it isn't live. * faces.h: Add some documentation on FACE_FONT. * frame-gtk.c (gtk_update_frame_external_traits): * frame-x.c (x_update_frame_external_traits): In the event that FACE_FONT has deleted the frame, don't manipulate it further in update_frame_external_traits. * mule-charset.c: * mule-charset.c (Fset_charset_registries): Don't allow XLFD wildcards in charset registries. Call the factored-out C-callable version to * objects-gtk.c: #include "charset.h" * objects-xlike-inc.c (x_find_charset_font, gtk_find_charset_font): In the event that the charset is ASCII and we haven't matched anything up to now, even with a pattern of "*", add "iso8859-1" to the charset's registry and try again. * window.c (window_pixel_width_to_char_width): The default width of a face may be zero; only divide by it if it's nonzero. XEmacs Trunk source patch: Diff command: cvs -q diff -Nu Files affected: src/window.c =================================================================== RCS src/objects-xlike-inc.c =================================================================== RCS src/objects-gtk.c =================================================================== RCS src/mule-charset.c =================================================================== RCS src/frame-x.c =================================================================== RCS src/frame-gtk.c =================================================================== RCS src/faces.h =================================================================== RCS src/faces.c =================================================================== RCS src/charset.h =================================================================== RCS Index: src/charset.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/charset.h,v retrieving revision 1.15 diff -u -u -r1.15 charset.h --- src/charset.h 2006/11/05 22:31:43 1.15 +++ src/charset.h 2006/11/11 12:31:00 ＠＠ -574,6 +574,8 ＠＠ int USED_IF_MULE (l), unsigned_char_dynarr *dst, enum unicode_type type, unsigned int little_endian); +void set_charset_registries(Lisp_Object charset, Lisp_Object registries); + EXFUN (Funicode_to_char, 2); EXFUN (Fchar_to_unicode, 1); Index: src/faces.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/faces.c,v retrieving revision 1.50 diff -u -u -r1.50 faces.c --- src/faces.c 2006/11/05 22:31:43 1.50 +++ src/faces.c 2006/11/11 12:31:08 ＠＠ -1162,7 +1162,7 ＠＠ /* Lookup the face again, this time allowing the fallback. If this succeeds, it'll give a font intended for the script in question, which is preferable to translating to ISO10646-1 and using the - fixed-with fallback. */ + fixed-width fallback. */ new_val = face_property_matching_instance (face, Qfont, charset, domain, ERROR_ME_DEBUG_WARN, 0, ＠＠ -1840,6 +1840,9 ＠＠ update_EmacsFrame (Lisp_Object frame, Lisp_Object name) { struct frame *frm = XFRAME (frame); + + if (!FRAME_LIVE_P(frm)) + return; if (EQ (name, Qfont)) MARK_FRAME_SIZE_SLIPPED (frm); Index: src/faces.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/faces.h,v retrieving revision 1.18 diff -u -u -r1.18 faces.h --- src/faces.h 2006/11/05 22:31:43 1.18 +++ src/faces.h 2006/11/11 12:31:11 ＠＠ -386,6 +386,10 ＠＠ FACE_PROPERTY_INSTANCE (face, Qforeground, domain, 0, Qzero) #define FACE_BACKGROUND(face, domain) \ FACE_PROPERTY_INSTANCE (face, Qbackground, domain, 0, Qzero) + +/* Calling this function on the default face with the ASCII character set + may delete any X11 frames; see the code at the end of + x_find_charset_font. */ #define FACE_FONT(face, domain, charset) \ face_property_matching_instance (face, Qfont, charset, domain, \ ERROR_ME_DEBUG_WARN, 0, Qzero, \ Index: src/frame-gtk.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/frame-gtk.c,v retrieving revision 1.22 diff -u -u -r1.22 frame-gtk.c --- src/frame-gtk.c 2005/11/25 01:42:02 1.22 +++ src/frame-gtk.c 2006/11/11 12:31:15 ＠＠ -1430,6 +1430,17 ＠＠ { Lisp_Object font = FACE_FONT (Vdefault_face, frame, Vcharset_ascii); + /* It may be that instantiating the font has deleted the frame (will + happen if the user has specified a charset registry for ASCII that + isn't available on the server, and our fallback of iso8859-1 isn't + available; something vanishingly rare.) In that case, return from + this function. */ + + if (!FRAME_LIVE_P(frm)) + { + return; + } + if (!EQ (font, Vthe_null_font_instance)) { /* #### BILL!!! The X code set the XtNfont property of the Index: src/frame-x.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/frame-x.c,v retrieving revision 1.75 diff -u -u -r1.75 frame-x.c --- src/frame-x.c 2006/06/19 18:49:23 1.75 +++ src/frame-x.c 2006/11/11 12:31:23 ＠＠ -2734,6 +2734,17 ＠＠ { Lisp_Object font = FACE_FONT (Vdefault_face, frame, Vcharset_ascii); + /* It may be that instantiating the font has deleted the frame (will + happen if the user has specified a charset registry for ASCII that + isn't available on the server, and our fallback of iso8859-1 isn't + available; something vanishingly rare.) In that case, return from + this function without further manipulation of the dead frame. */ + + if (!FRAME_LIVE_P(frm)) + { + return; + } + /* #### what to do about Xft? I don't think the font is actually used to compute cell size for computing frame pixel dimensions (see call to EmacsFrameRecomputeCellSize() below); where is it used? -- sjt Index: src/mule-charset.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/mule-charset.c,v retrieving revision 1.50 diff -u -u -r1.50 mule-charset.c --- src/mule-charset.c 2006/11/07 14:04:56 1.50 +++ src/mule-charset.c 2006/11/11 12:31:25 ＠＠ -883,6 +883,14 ＠＠ return Qnil; } +void +set_charset_registries(Lisp_Object charset, Lisp_Object registries) +{ + XCHARSET_REGISTRIES (charset) = registries; + invalidate_charset_font_caches (charset); + face_property_was_changed (Vdefault_face, Qfont, Qglobal); +} + DEFUN ("set-charset-registries", Fset_charset_registries, 2, 2, 0, /* Set the `registries' property of CHARSET to REGISTRIES. ＠＠ -913,11 +921,19 ＠＠ invalid_argument("Not an X11 REGISTRY-ENCODING combination", XVECTOR_DATA(registries)[i]); } + + if (qxestrchr(XSTRING_DATA(XVECTOR_DATA(registries)[i]), '*') || + qxestrchr(XSTRING_DATA(XVECTOR_DATA(registries)[i]), '?')) + { + invalid_argument + ("XLFD wildcards not allowed in charset-registries", + XVECTOR_DATA(registries)[i]); + + } } - XCHARSET_REGISTRIES (charset) = registries; - invalidate_charset_font_caches (charset); - face_property_was_changed (Vdefault_face, Qfont, Qglobal); + set_charset_registries(charset, registries); + return Qnil; } Index: src/objects-gtk.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/objects-gtk.c,v retrieving revision 1.18 diff -u -u -r1.18 objects-gtk.c --- src/objects-gtk.c 2006/11/05 22:31:44 1.18 +++ src/objects-gtk.c 2006/11/11 12:31:28 ＠＠ -31,6 +31,7 ＠＠ #include "lisp.h" #include "buffer.h" +#include "charset.h" #include "device-impl.h" #include "insdel.h" Index: src/objects-xlike-inc.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/objects-xlike-inc.c,v retrieving revision 1.3 diff -u -u -r1.3 objects-xlike-inc.c --- src/objects-xlike-inc.c 2006/11/05 22:31:45 1.3 +++ src/objects-xlike-inc.c 2006/11/11 12:31:29 ＠＠ -764,6 +764,92 ＠＠ charset, stage); } + /* In the event that the charset is ASCII and we haven't matched + anything up to now, even with a pattern of "*", add "iso8859-1" + to the charset's registry and try again. Not returning a result + for ASCII means our frame geometry calculations are + inconsistent, and that we may crash. */ + + if (1 == xlfd_length && EQ(charset, Vcharset_ascii) && NILP(result) + && ('*' == eigetch(ei_xlfd_without_registry, 0))) + + { + int have_latin1 = 0; + + /* Set this to, for example, is08859-1 if you want to see the + error behaviour. */ + +#define FALLBACK_ASCII_REGISTRY "iso8859-1" + + for (j = 0; j < registries_len; ++j) + { + if (0 == qxestrcasecmp(XSTRING_DATA(XVECTOR_DATA(registries)[j]), + FALLBACK_ASCII_REGISTRY)) + { + have_latin1 = 1; + break; + } + } + + if (!have_latin1) + { + Lisp_Object new_registries = make_vector(registries_len + 1, Qnil); + + warn_when_safe (Qface, Qwarning, + "Your ASCII charset registries contain nothing " + "sensible. Adding `" FALLBACK_ASCII_REGISTRY "'."); + + XVECTOR_DATA(new_registries)[0] + = build_string(FALLBACK_ASCII_REGISTRY); + + memcpy(XVECTOR_DATA(new_registries) + 1, + XVECTOR_DATA(registries), + sizeof XVECTOR_DATA(registries)[0] * + XVECTOR_LENGTH(registries)); + + /* Calling set_charset_registries instead of overwriting the + value directly, to allow the charset font caches to be + invalidated and a change to the default face to be + noted. */ + set_charset_registries(charset, new_registries); + + /* And recurse. */ + result = + DEVMETH_OR_GIVEN (XDEVICE (device), find_charset_font, + (device, font, charset, stage), + result); + } + else + { + DECLARE_EISTRING (ei_connection_name); + + /* We preserve a copy of the connection name for the error message + after the device is deleted. */ + eicpy_lstr (ei_connection_name, + DEVICE_CONNECTION (XDEVICE(device))); + + stderr_out ("Cannot find a font for ASCII, deleting device on %s\n", + eidata (ei_connection_name)); + + io_error_delete_device (device); + + /* Do a normal warning in the event that we have other, non-X + frames available. (If we don't, io_error_delete_device will + have exited.) */ + warn_when_safe + (Qface, Qerror, + "Cannot find a font for ASCII, deleting device on %s.\n" + "\n" + "Your X server fonts appear to be inconsistent; fix them, or\n" + "the next frame you create on that DISPLAY will crash this\n" + "XEmacs. At a minimum, provide one font with an XLFD ending\n" + "in `" FALLBACK_ASCII_REGISTRY "', so we can work out what size\n" + "a frame should be. ", + eidata (ei_connection_name)); + } + + } + /* This function used to return the font spec, in the case where a font didn't exist on the X server but it did match the charset. We're not doing that any more, because none of the other platform code does, and Index: src/window.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/window.c,v retrieving revision 1.92 diff -u -u -r1.92 window.c --- src/window.c 2006/06/21 17:30:37 1.92 +++ src/window.c 2006/11/11 12:31:40 ＠＠ -4250,7 +4250,7 ＠＠ int include_margins_p) { int avail_width; - int char_width; + int char_width = 0; int defheight, defwidth; Lisp_Object window = wrap_window (w); ＠＠ -4263,7 +4263,8 ＠＠ default_face_height_and_width (window, &defheight, &defwidth); - char_width = (avail_width / defwidth); + if (defwidth) + char_width = (avail_width / defwidth); /* It's the calling function's responsibility to check these values and make sure they're not out of range. -- Santa Maradona, priez pour moi! _______________________________________________ XEmacs-Patches mailing list XEmacs-Patches(a)xemacs.org http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-patches