Subject: Re: bug#11984: 24.1; segfault while deleting a window

From: martin rudalics <[email-masked]>
Date: Tue, 24 Jul 2012 14:46:46 +0200

> IMHO the right fix is to throw an error for
 >
 >   if (NILP (w->buffer) && NILP (w->hchild) && NILP (w->vchild))
 >
 > which means that some functions when called on dead windows (like
 > `delete-window') will now throw an error.  These will have to be caught
 > on the Elisp level.

I think `delete-window-internal' can keep its decode_any_window.  I'm
not sure about `window-buffer'.  For the remainder, the attached patch
should be applied IMO.  Eli can you check whether this interferes with
tooltip creation?

martin
=== modified file 'src/lisp.h'
--- src/lisp.h	2012-07-24 06:45:44 +0000
+++ src/lisp.h	2012-07-24 08:32:47 +0000
@@ -1732,6 +1732,13 @@
 #define CHECK_LIVE_WINDOW(x) \
   CHECK_TYPE (WINDOWP (x) && !NILP (XWINDOW (x)->buffer), Qwindow_live_p, x)
 
+#define CHECK_VALID_WINDOW(x) \
+  CHECK_TYPE (WINDOWP (x)				\
+	      && (!NILP (XWINDOW (x)->buffer)		\
+		  || !NILP (XWINDOW (x)->vchild)	\
+		  || !NILP (XWINDOW (x)->hchild)),	\
+	      Qwindow_valid_p, x)
+
 #define CHECK_PROCESS(x) \
   CHECK_TYPE (PROCESSP (x), Qprocessp, x)

=== modified file 'src/window.c'
--- src/window.c	2012-07-23 16:57:20 +0000
+++ src/window.c	2012-07-24 10:33:45 +0000
@@ -51,7 +51,7 @@
 #include "nsterm.h"
 #endif
 
-Lisp_Object Qwindowp, Qwindow_live_p;
+Lisp_Object Qwindowp, Qwindow_live_p, Qwindow_valid_p;
 static Lisp_Object Qwindow_configuration_p, Qrecord_window_buffer;
 static Lisp_Object Qwindow_deletable_p, Qdelete_window, Qdisplay_buffer;
 static Lisp_Object Qreplace_buffer_in_windows, Qget_mru_window;
@@ -156,6 +156,19 @@
   return w;
 }
 
+static struct window *
+decode_valid_window (register Lisp_Object window)
+{
+  struct window *w;
+
+  if (NILP (window))
+    return XWINDOW (selected_window);
+
+  CHECK_VALID_WINDOW (window);
+  w = XWINDOW (window);
+  return w;
+}
+
 DEFUN ("windowp", Fwindowp, Swindowp, 1, 1, 0,
        doc: /* Return t if OBJECT is a window and nil otherwise.  */)
   (Lisp_Object object)
@@ -178,7 +191,7 @@
 If WINDOW is omitted or nil, it defaults to the selected window.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->frame;
+  return decode_valid_window (window)->frame;
 }
 
 DEFUN ("frame-root-window", Fframe_root_window, Sframe_root_window, 0, 1, 0,
@@ -193,7 +206,10 @@
   if (NILP (frame_or_window))
     window = SELECTED_FRAME ()->root_window;
   else if (WINDOWP (frame_or_window))
-    window = XFRAME (WINDOW_FRAME (XWINDOW (frame_or_window)))->root_window;
+    {
+      CHECK_VALID_WINDOW (frame_or_window);
+      window = XFRAME (XWINDOW (frame_or_window)->frame)->root_window;
+    }
   else
     {
       CHECK_LIVE_FRAME (frame_or_window);
@@ -220,7 +236,7 @@
 If WINDOW is omitted or nil, it defaults to the selected window.  */)
   (Lisp_Object window)
 {
-  return MINI_WINDOW_P (decode_any_window (window)) ? Qt : Qnil;
+  return MINI_WINDOW_P (decode_valid_window (window)) ? Qt : Qnil;
 }
 
 /* Don't move this to window.el - this must be a safe routine.  */
@@ -427,7 +443,7 @@
 Return nil for a window with no parent (e.g. a root window).  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->parent;
+  return decode_valid_window (window)->parent;
 }
 
 DEFUN ("window-top-child", Fwindow_top_child, Swindow_top_child, 1, 1, 0,
@@ -438,7 +454,7 @@
   (Lisp_Object window)
 {
   CHECK_WINDOW (window);
-  return decode_any_window (window)->vchild;
+  return decode_valid_window (window)->vchild;
 }
 
 DEFUN ("window-left-child", Fwindow_left_child, Swindow_left_child, 1, 1, 0,
@@ -449,7 +465,7 @@
   (Lisp_Object window)
 {
   CHECK_WINDOW (window);
-  return decode_any_window (window)->hchild;
+  return decode_valid_window (window)->hchild;
 }
 
 DEFUN ("window-next-sibling", Fwindow_next_sibling, Swindow_next_sibling, 0, 1, 0,
@@ -458,7 +474,7 @@
 Return nil if WINDOW has no next sibling.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->next;
+  return decode_valid_window (window)->next;
 }
 
 DEFUN ("window-prev-sibling", Fwindow_prev_sibling, Swindow_prev_sibling, 0, 1, 0,
@@ -467,7 +483,7 @@
 Return nil if WINDOW has no previous sibling.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->prev;
+  return decode_valid_window (window)->prev;
 }
 
 DEFUN ("window-combination-limit", Fwindow_combination_limit, Swindow_combination_limit, 1, 1, 0,
@@ -477,7 +493,7 @@
 WINDOW are never \(re-)combined with WINDOW's siblings.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->combination_limit;
+  return decode_valid_window (window)->combination_limit;
 }
 
 DEFUN ("set-window-combination-limit", Fset_window_combination_limit, Sset_window_combination_limit, 2, 2, 0,
@@ -488,7 +504,7 @@
 for future use.  */)
   (Lisp_Object window, Lisp_Object limit)
 {
-  register struct window *w = decode_any_window (window);
+  register struct window *w = decode_valid_window (window);
   w->combination_limit = limit;
   return w->combination_limit;
 }
@@ -516,7 +532,7 @@
 integer multiple of the default character height.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->total_lines;
+  return decode_valid_window (window)->total_lines;
 }
 
 DEFUN ("window-total-width", Fwindow_total_width, Swindow_total_width, 0, 1, 0,
@@ -531,7 +547,7 @@
 integer multiple of the default character width.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->total_cols;
+  return decode_valid_window (window)->total_cols;
 }
 
 DEFUN ("window-new-total", Fwindow_new_total, Swindow_new_total, 0, 1, 0,
@@ -539,7 +555,7 @@
 If WINDOW is omitted or nil, it defaults to the selected window.   */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->new_total;
+  return decode_valid_window (window)->new_total;
 }
 
 DEFUN ("window-normal-size", Fwindow_normal_size, Swindow_normal_size, 0, 2, 0,
@@ -549,9 +565,9 @@
   (Lisp_Object window, Lisp_Object horizontal)
 {
   if (NILP (horizontal))
-    return decode_any_window (window)->normal_lines;
+    return decode_valid_window (window)->normal_lines;
   else
-    return decode_any_window (window)->normal_cols;
+    return decode_valid_window (window)->normal_cols;
 }
 
 DEFUN ("window-new-normal", Fwindow_new_normal, Swindow_new_normal, 0, 1, 0,
@@ -559,7 +575,7 @@
 If WINDOW is omitted or nil, it defaults to the selected window.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->new_normal;
+  return decode_valid_window (window)->new_normal;
 }
 
 DEFUN ("window-left-column", Fwindow_left_column, Swindow_left_column, 0, 1, 0,
@@ -571,7 +587,7 @@
 If WINDOW is omitted or nil, it defaults to the selected window.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->left_col;
+  return decode_valid_window (window)->left_col;
 }
 
 DEFUN ("window-top-line", Fwindow_top_line, Swindow_top_line, 0, 1, 0,
@@ -583,7 +599,7 @@
 If WINDOW is omitted or nil, it defaults to the selected window.  */)
   (Lisp_Object window)
 {
-  return decode_any_window (window)->top_line;
+  return decode_valid_window (window)->top_line;
 }
 
 /* Return the number of lines of W's body.  Don't count any mode or
@@ -751,7 +767,7 @@
 just the text area, use `window-inside-edges'.  */)
   (Lisp_Object window)
 {
-  register struct window *w = decode_any_window (window);
+  register struct window *w = decode_valid_window (window);
 
   return Fcons (make_number (WINDOW_LEFT_EDGE_COL (w)),
 	 Fcons (make_number (WINDOW_TOP_EDGE_LINE (w)),
@@ -772,7 +788,7 @@
 of just the text area, use `window-inside-pixel-edges'.  */)
   (Lisp_Object window)
 {
-  register struct window *w = decode_any_window (window);
+  register struct window *w = decode_valid_window (window);
 
   return Fcons (make_number (WINDOW_LEFT_EDGE_X (w)),
 	 Fcons (make_number (WINDOW_TOP_EDGE_Y (w)),
@@ -816,7 +832,7 @@
 of just the text area, use `window-inside-absolute-pixel-edges'.  */)
   (Lisp_Object window)
 {
-  register struct window *w = decode_any_window (window);
+  register struct window *w = decode_valid_window (window);
   int add_x, add_y;
   calc_absolute_offset (w, &add_x, &add_y);
 
@@ -1700,7 +1716,7 @@
 elements of the form (PARAMETER . VALUE).  */)
   (Lisp_Object window)
 {
-  return Fcopy_alist (decode_any_window (window)->window_parameters);
+  return Fcopy_alist (decode_valid_window (window)->window_parameters);
 }
 
 DEFUN ("window-parameter", Fwindow_parameter, Swindow_parameter,
@@ -1711,7 +1727,7 @@
 {
   Lisp_Object result;
 
-  result = Fassq (parameter, decode_any_window (window)->window_parameters);
+  result = Fassq (parameter, decode_valid_window (window)->window_parameters);
   return CDR_SAFE (result);
 }
 
@@ -1721,7 +1737,7 @@
 WINDOW defaults to the selected window.  Return VALUE.  */)
   (Lisp_Object window, Lisp_Object parameter, Lisp_Object value)
 {
-  register struct window *w = decode_any_window (window);
+  register struct window *w = decode_valid_window (window);
   Lisp_Object old_alist_elt;
 
   old_alist_elt = Fassq (parameter, w->window_parameters);
@@ -2585,7 +2601,7 @@
   ptrdiff_t startpos IF_LINT (= 0);
   int top IF_LINT (= 0), new_top, resize_failed;
 
-  w = decode_any_window (window);
+  w = decode_valid_window (window);
   XSETWINDOW (window, w);
   f = XFRAME (w->frame);
 
@@ -2598,7 +2614,7 @@
   else
     /* ROOT must be an ancestor of WINDOW.  */
     {
-      r = decode_any_window (root);
+      r = decode_valid_window (root);
       pwindow = XWINDOW (window)->parent;
       while (!NILP (pwindow))
 	if (EQ (pwindow, root))
@@ -3325,7 +3341,7 @@
 Note: This function does not operate on any child windows of WINDOW.  */)
      (Lisp_Object window, Lisp_Object size, Lisp_Object add)
 {
-  struct window *w = decode_any_window (window);
+  struct window *w = decode_valid_window (window);
 
   CHECK_NUMBER (size);
   if (NILP (add))
@@ -3343,7 +3359,7 @@
 Note: This function does not operate on any child windows of WINDOW.  */)
      (Lisp_Object window, Lisp_Object size)
 {
-  struct window *w = decode_any_window (window);
+  struct window *w = decode_valid_window (window);
 
   w->new_normal = size;
   return w->new_normal;
@@ -6491,6 +6507,7 @@
   DEFSYM (Qwindowp, "windowp");
   DEFSYM (Qwindow_configuration_p, "window-configuration-p");
   DEFSYM (Qwindow_live_p, "window-live-p");
+  DEFSYM (Qwindow_valid_p, "window-valid-p");
   DEFSYM (Qwindow_deletable_p, "window-deletable-p");
   DEFSYM (Qdelete_window, "delete-window");
   DEFSYM (Qwindow_resize_root_window, "window--resize-root-window");