[please CC me on replies; I am no longer on xemacs-beta]
Hi,
In trying to come up with a recipe to reliably exhibit the crash I
reported as issue #331:
http://tracker.xemacs.org/XEmacs/its/issue331
I've come across what I believe is a bug in the window_loop function
used by GET-BUFFER-WINDOW and several other elisp primitives defined
in src/window.c.
The documentation for GET-BUFFER-WINDOW says the following about the
optional WHICH-DEVICES argument:
| If nil or omitted, search all devices on the selected console.
| [...]
| If a console, search all devices on that console.
I interpret that to mean either of these forms:
(get-buffer-window "foo" 'visible)
(get-buffer-window "foo" 'visible NIL)
should be equivalent to:
(get-buffer-window "foo" 'visible (selected-console))
However, this is not the case: the first two forms will return windows
on consoles other than the selected console.
Looking at the code, the problem seems pretty clear. Just inside the
device loop is the following code:
| if (frame)
| XSETFRAME (the_frame, frame);
| else
| the_frame = DEVICE_SELECTED_FRAME (XDEVICE (device));
|
| if (NILP (the_frame))
| continue;
|
| if (!device_matches_device_spec (device,
| NILP (which_devices) ?
| FRAME_CONSOLE (XFRAME (the_frame)) :
| which_devices))
| continue;
So when FRAME is not set, and WHICH_DEVICES is NIL, we check the
device currently being considered against that same device's selected
frame's console, which will tautologically match.
Since FRAME is not set when WHICH_FRAMES is 'VISIBLE, 0, or T, that is
exactly when the bug is manifested.
Fixing this seems a simple matter of adding the line:
| if (NILP (which_devices))
| which_devices = selected_frame ();
toward the top of the function, and changing the device check to:
| if (!device_matches_device_spec (device, which_devices))
| continue;
I can test this change and submit a proper patch if someone will
confirm that this analysis sounds right.
The code in question was last touched in April 2001 by Michael
Sperber, however that appears to have just been some sort of merge,
the commit message is:
| The Great Trunk Move from release-21-4.
The code was previously changed by Martin Buchholz in November 2000
with the commit message
| next-frame crashes fixed
and that appears to be the change that introduced this bug.
Incidentally, the other use of THE_FRAME just below the code cited:
| /* Pick a window to start with. */
| if (WINDOWP (obj))
| w = obj;
| else
| w = FRAME_SELECTED_WINDOW (XFRAME (the_frame));
does seem to be correct.
Back to issue #331, while I have reproduced the original crash once, I
unfortunately cannot yet do so reliably. I do suspect that fixing the
bug described here would avoid the crash in the majority of cases,
though I believe it would still be possible when WHICH-DEVICES is T.
thanks for your time,
Greg
_______________________________________________
XEmacs-Beta mailing list
XEmacs-Beta(a)xemacs.org
http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-beta