From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [RFC] Getting rid of selected_window Date: Fri, 29 Nov 2013 17:19:38 +0200 Message-ID: <83fvqf6xrp.fsf@gnu.org> References: <5298804A.5040405@yandex.ru> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: ger.gmane.org 1385738451 11076 80.91.229.3 (29 Nov 2013 15:20:51 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 29 Nov 2013 15:20:51 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dmitry Antipov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Fri Nov 29 16:20:55 2013 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1VmPsE-0002Nb-P5 for ged-emacs-devel@m.gmane.org; Fri, 29 Nov 2013 16:20:54 +0100 Original-Received: from localhost ([::1]:47865 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VmPsE-0006Rc-C9 for ged-emacs-devel@m.gmane.org; Fri, 29 Nov 2013 10:20:54 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:33046) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VmPs6-0006RW-5K for emacs-devel@gnu.org; Fri, 29 Nov 2013 10:20:52 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VmPrz-0007Cb-Vf for emacs-devel@gnu.org; Fri, 29 Nov 2013 10:20:46 -0500 Original-Received: from mtaout20.012.net.il ([80.179.55.166]:57658) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VmPrz-0007CK-GQ for emacs-devel@gnu.org; Fri, 29 Nov 2013 10:20:39 -0500 Original-Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0MX1007006JO9M00@a-mtaout20.012.net.il> for emacs-devel@gnu.org; Fri, 29 Nov 2013 17:19:48 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0MX1007NH6L01N90@a-mtaout20.012.net.il>; Fri, 29 Nov 2013 17:19:48 +0200 (IST) In-reply-to: <5298804A.5040405@yandex.ru> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-Received-From: 80.179.55.166 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:165878 Archived-At: > Date: Fri, 29 Nov 2013 15:53:46 +0400 > From: Dmitry Antipov > > I'm trying to get rid of selected_window, with an intent to completely cleanup corner > cases where FRAME_SELECTED_WINDOW (selected_frame) is not the same as selected_window > and avoid redundant synchronization of them. What for? I don't think you can get rid of the need to synchronize those, because of features like focus redirection and minibuffer-only frames. > Comments are welcome. Here are some: > === modified file 'src/cmds.c' > --- src/cmds.c 2013-09-05 02:27:13 +0000 > +++ src/cmds.c 2013-11-29 11:02:47 +0000 > @@ -282,7 +282,7 @@ > nonundocount = 0; > > if (NILP (Vexecuting_kbd_macro) > - && !EQ (minibuf_window, selected_window)) > + && !EQ (minibuf_window, Fselected_window ())) > { > if (nonundocount <= 0 || nonundocount >= 20) > { > > === modified file 'src/dispextern.h' > --- src/dispextern.h 2013-11-06 00:14:56 +0000 > +++ src/dispextern.h 2013-11-29 11:02:47 +0000 > @@ -1410,7 +1410,7 @@ > > #define CURRENT_MODE_LINE_FACE_ID_3(SELW, MBW, SCRW) \ > ((!mode_line_in_non_selected_windows \ > - || (SELW) == XWINDOW (selected_window) \ > + || (SELW) == SELECTED_WINDOW () \ I don't understand what is the difference between Fselected_window and SELECTED_WINDOW. Why do we use both in C? It's confusing. > --- src/editfns.c 2013-11-29 01:22:40 +0000 > +++ src/editfns.c 2013-11-29 11:02:47 +0000 > @@ -832,8 +832,8 @@ > ? Fcopy_marker (BVAR (current_buffer, mark), Qnil) > : Qnil), > /* Selected window if current buffer is shown in it, nil otherwise. */ > - (EQ (XWINDOW (selected_window)->contents, Fcurrent_buffer ()) > - ? selected_window : Qnil), > + (EQ (SELECTED_WINDOW ()->contents, Fcurrent_buffer ()) > + ? Fselected_window () : Qnil), SELECTED_WINDOW can return NULL, and then this dereference will segfault. > --- src/fileio.c 2013-11-27 16:08:53 +0000 > +++ src/fileio.c 2013-11-29 11:13:44 +0000 > @@ -3868,8 +3868,8 @@ > > /* If display currently starts at beginning of line, > keep it that way. */ > - if (XBUFFER (XWINDOW (selected_window)->contents) == current_buffer) > - XWINDOW (selected_window)->start_at_line_beg = !NILP (Fbolp ()); > + if (SELECTED_BUFFER () == current_buffer) > + SELECTED_WINDOW ()->start_at_line_beg = !NILP (Fbolp ()); Same here (and elsewhere in the patch). > +#define SELECTED_WINDOW() \ > + ((FRAMEP (selected_frame) && FRAME_LIVE_P (XFRAME (selected_frame))) \ > + ? XWINDOW (FRAME_SELECTED_WINDOW (XFRAME (selected_frame))) : NULL) This changes the semantics of selected_window, because it requires that its frame be live. I don't think we understand the effects of this change. > +/* This is the buffer displayed by the window above. > + Note that this is not the same as current_buffer. */ > + > +#define SELECTED_BUFFER() \ > + (eassert (SELECTED_WINDOW ()), XBUFFER (SELECTED_WINDOW ()->contents)) So now we have SELECTED_BUFFER and current_buffer, which are different in unspecified ways. What do you think is the probability that someone will use the wrong one of that pair? I think it's 100%. > /* do_pending_window_change could change the selected_window due to > frame resizing which makes the selected window too small. */ > - if (WINDOWP (selected_window) && (w = XWINDOW (selected_window)) != sw) > + if ((w = SELECTED_WINDOW ()) != sw) > sw = w; What if SELECTED_WINDOW returns NULL? You have now assigned NULL to sw. The old code did better. > @@ -13117,7 +13106,7 @@ > && minibuf_level == 0 > /* If the mini-window is currently selected, this means the > echo-area doesn't show through. */ > - && !MINI_WINDOW_P (XWINDOW (selected_window)))) > + && !MINI_WINDOW_P (SELECTED_WINDOW ()))) MINI_WINDOW_P will segfault if SELECTED_WINDOW returns NULL. > @@ -18115,11 +18101,10 @@ > GLYPH > 1 or omitted means dump glyphs in long form. */) > (Lisp_Object row, Lisp_Object glyphs) > { > - struct glyph_matrix *matrix; > + struct glyph_matrix *matrix = SELECTED_WINDOW ()->current_matrix; > EMACS_INT vpos; > > CHECK_NUMBER (row); > - matrix = XWINDOW (selected_window)->current_matrix; This will segfault where the old code didn't (if 'row' was not a number).