this looks like a bug in GET-BUFFER-WINDOW..

[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