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: 'struct window' cleanup #4 Date: Mon, 02 Jul 2012 20:33:49 +0300 Message-ID: <83ehou5apu.fsf@gnu.org> References: <4FF17BAE.7090200@yandex.ru> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org X-Trace: dough.gmane.org 1341250454 1188 80.91.229.3 (2 Jul 2012 17:34:14 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 2 Jul 2012 17:34:14 +0000 (UTC) Cc: emacs-devel@gnu.org To: Dmitry Antipov Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Mon Jul 02 19:34:13 2012 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 1SlkVl-0008Bu-Bk for ged-emacs-devel@m.gmane.org; Mon, 02 Jul 2012 19:34:09 +0200 Original-Received: from localhost ([::1]:59914 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlkVk-00014N-AJ for ged-emacs-devel@m.gmane.org; Mon, 02 Jul 2012 13:34:08 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:48394) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlkVh-0000zq-02 for emacs-devel@gnu.org; Mon, 02 Jul 2012 13:34:06 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SlkVY-0007ib-1F for emacs-devel@gnu.org; Mon, 02 Jul 2012 13:34:04 -0400 Original-Received: from mtaout20.012.net.il ([80.179.55.166]:63441) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SlkVX-0007iE-Q5 for emacs-devel@gnu.org; Mon, 02 Jul 2012 13:33:55 -0400 Original-Received: from conversion-daemon.a-mtaout20.012.net.il by a-mtaout20.012.net.il (HyperSendmail v2007.08) id <0M6J00000NFTR400@a-mtaout20.012.net.il> for emacs-devel@gnu.org; Mon, 02 Jul 2012 20:33:54 +0300 (IDT) Original-Received: from HOME-C4E4A596F7 ([87.69.210.75]) by a-mtaout20.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0M6J00N3CNGH2YE0@a-mtaout20.012.net.il>; Mon, 02 Jul 2012 20:33:54 +0300 (IDT) In-reply-to: <4FF17BAE.7090200@yandex.ru> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: Solaris 10 (beta) 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:151370 Archived-At: > Date: Mon, 02 Jul 2012 14:45:02 +0400 > From: Dmitry Antipov > > --- src/dispnew.c 2012-06-28 07:50:50 +0000 > +++ src/dispnew.c 2012-07-02 10:11:48 +0000 > @@ -643,9 +643,8 @@ > > /* Window end is invalid, if inside of the rows that > are invalidated below. */ > - if (INTEGERP (w->window_end_vpos) > - && XFASTINT (w->window_end_vpos) >= i) > - w->window_end_valid = Qnil; > + if (w->window_end_vpos >= i) > + w->window_end_valid = 0; Shouldn't we test w->window_end_valid for being non-zero, before we trust the value of w->window_end_vpos? Zero for a vertical position is perfectly valid, so if i is zero, you can err here. The comment says: /* Glyph matrix row of the last glyph in the current matrix of W. Should be nonnegative, and valid only if window_end_valid is not zero. */ ptrdiff_t window_end_vpos; So I think we should put our money where our mouth is, and do as we say ;-) > @@ -2989,10 +2989,10 @@ > XSETINT (BVAR (b, display_count), XINT (BVAR (b, display_count)) + 1); > BVAR (b, display_time) = Fcurrent_time (); > > - XSETFASTINT (w->window_end_pos, 0); > - XSETFASTINT (w->window_end_vpos, 0); > + w->window_end_pos = 0; > + w->window_end_vpos = 0; > memset (&w->last_cursor, 0, sizeof w->last_cursor); > - w->window_end_valid = Qnil; > + w->window_end_valid = 0; > if (!(keep_margins_p && samebuf)) > { /* If we're not actually changing the buffer, don't reset hscroll and > vscroll. This case happens for example when called from > @@ -3287,8 +3287,6 @@ > w->start = Fmake_marker (); > w->pointm = Fmake_marker (); > w->vertical_scroll_bar_type = Qt; > - XSETFASTINT (w->window_end_pos, 0); > - XSETFASTINT (w->window_end_vpos, 0); > > /* Initialize non-Lisp data. Note that allocate_window zeroes out all > non-Lisp data, so do it only for slots which should not be zero. */ > @@ -3296,6 +3294,8 @@ > w->phys_cursor_type = -1; > w->phys_cursor_width = -1; > w->sequence_number = ++sequence_number; > + w->window_end_pos = -1; > + w->window_end_vpos = -1; If the "invalid" value is going to be -1, then why did you initialize with zero above? > + /* Z - the buffer position of the last glyph in the current matrix of W. > + Should be nonnegative, and valid only if window_end_valid is not zero. */ > + ptrdiff_t window_end_pos; But since you initialize with -1, the "nonnegative" part is only true when window_end_valid is non-zero, right? > + /* Glyph matrix row of the last glyph in the current matrix of W. > + Should be nonnegative, and valid only if window_end_valid is not zero. */ > + ptrdiff_t window_end_vpos; Same here. > @@ -27410,7 +27419,7 @@ > And verify the buffer's text has not changed. */ > b = XBUFFER (w->buffer); > if (part == ON_TEXT > - && EQ (w->window_end_valid, w->buffer) > + && w->window_end_valid Bother: doesn't the comparison with w->buffer test for a specific buffer, rather than just non-nil value? The corresponding assignment, viz.: > if (accurate_p) > { > - w->window_end_valid = w->buffer; > + w->window_end_valid = 1; > w->update_mode_line = 0; > } seems to handle the case when the display is "accurate". Perhaps use a different value, like 2, for this situation? Other than that, looks okay, thanks.