bug#18528: 24.3.93; Crash during restoration of frameset from desktop

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

Today I started Emacs 24.3.93, and it crashed near the end of
restoring the last session from .emacs.desktop, when it was
re-creating the frames recorded in that file.  Here's the backtrace:

  Program received signal SIGSEGV, Segmentation fault.
  _malloc_internal_nolock (size=size@entry=4294967285) at gmalloc.c:897
  897     gmalloc.c: No such file or directory.
  (gdb) bt 10
  #0  _malloc_internal_nolock (size=size@entry=4294967285) at gmalloc.c:897
  #1  0x011eff12 in _realloc_internal_nolock (ptr=0x3e89600, size=4294967285)
      at gmalloc.c:1441
  #2  0x01123f22 in xrealloc (block=0x3e89600, size=4294967285) at alloc.c:717
  #3  0x0100a25e in adjust_decode_mode_spec_buffer (f=<optimized out>)
      at dispnew.c:2106
  #4  adjust_frame_glyphs (f=f@entry=0x418ca80) at dispnew.c:1756
  #5  0x0100abd0 in change_frame_size_1 (f=0x418ca80,
      new_width=<optimized out>, new_height=<optimized out>,
      pretend=pretend@entry=false, delay=delay@entry=false,
      safe=safe@entry=false, pixelwise=pixelwise@entry=true) at dispnew.c:5596
  #6  0x0100cc89 in change_frame_size (pixelwise=true, safe=<optimized out>,
      delay=false, pretend=false, new_height=<optimized out>,
      new_width=<optimized out>, f=<optimized out>) at dispnew.c:5471
  #7  do_pending_window_change (safe=safe@entry=false) at dispnew.c:5432
  #8  0x0100e9e8 in Fset_frame_size (frame=frame@entry=104658605, width=2880,
      height=3740, pixelwise=65550402) at frame.c:2645
  #9  0x010126ad in x_set_frame_parameters (f=f@entry=0x63cf6a8,
      alist=<optimized out>) at frame.c:3002
  (More stack frames follow...)
  (gdb) frame 4
  #4  adjust_frame_glyphs (f=f@entry=0x418ca80) at dispnew.c:1756
  1756    in dispnew.c
  (gdb) p/x f
  $5 = 0x418ca80
  (gdb) p f->text_cols
  $6 = -3  <<<<<<<<<<<<<<<<<<<

As you can see, the text_cols member is negative.  This is the
immediate cause of the crash, because adjust_decode_mode_spec_buffer
does this:

  static void
  adjust_decode_mode_spec_buffer (struct frame *f)
  {
    f->decode_mode_spec_buffer = xrealloc (f->decode_mode_spec_buffer,
					   FRAME_MESSAGE_BUF_SIZE (f) + 1);
  }

and FRAME_MESSAGE_BUF_SIZE is defined like this:

  #define FRAME_MESSAGE_BUF_SIZE(f) (((int) FRAME_COLS (f)) * 4)

So we pass a negative value to xrealloc, which interprets it as a very
large positive value, with predictable results.

Some digging into this reveals the following:

  . The negative values come from w32term.c, around line 4770, where
    they are derived from the value returned by GetClientRect.
    Evidently, it sometimes returns a (0, 0, 0, 0) rectangle for the
    frame dimensions, from which we then subtract the dimensions of
    frame decorations, like scroll bar etc., and call
    change_frame_size.  (We also don't check errors returned by
    GetClientRect.)

  . change_frame_size internally validates the requested dimensions,
    and doesn't allow them to become too small.  But it does that on
    pixel dimensions, and if those are corrected, the character-unit
    dimensions are not recalculated to reflect those corrections.

Below please find a patch that I intend to commit to the emacs-24
branch if no one objects.  Martin, I'd appreciate your review,
especially for the dispnew.c parts.

TIA

--- src/w32term.c~0	2014-05-24 23:48:43 +0300
+++ src/w32term.c	2014-09-21 17:48:00 +0300
@@ -4754,34 +4754,42 @@ w32_read_socket (struct terminal *termin
 	      RECT rect;
 	      int rows, columns, width, height, text_width, text_height;
 
-	      GetClientRect (msg.msg.hwnd, &rect);
-
-	      height = rect.bottom - rect.top;
-	      width = rect.right - rect.left;
-	      text_width = FRAME_PIXEL_TO_TEXT_WIDTH (f, width);
-	      text_height = FRAME_PIXEL_TO_TEXT_HEIGHT (f, height);
-	      rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (f, height);
-	      columns = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (f, width);
-
-	      /* TODO: Clip size to the screen dimensions.  */
-
-	      /* Even if the number of character rows and columns has
-		 not changed, the font size may have changed, so we need
-		 to check the pixel dimensions as well.  */
-
-	      if (width != FRAME_PIXEL_WIDTH (f)
-		  || height != FRAME_PIXEL_HEIGHT (f)
-		  || text_width != FRAME_TEXT_WIDTH (f)
-		  || text_height != FRAME_TEXT_HEIGHT (f))
+	      if (GetClientRect (msg.msg.hwnd, &rect)
+		  /* GetClientRect evidently returns (0, 0, 0, 0) if
+		     called on a minimized frame.  Such "dimensions"
+		     aren't useful anyway.  */
+		  && !(rect.bottom == 0
+		       && rect.top == 0
+		       && rect.left == 0
+		       && rect.right == 0))
 		{
-		  change_frame_size (f, text_width, text_height, 0, 1, 0, 1);
-		  SET_FRAME_GARBAGED (f);
-		  cancel_mouse_face (f);
-		  /* Do we want to set these here ????  */
-/** 		  FRAME_PIXEL_WIDTH (f) = width; **/
-/** 		  FRAME_TEXT_WIDTH (f) = text_width; **/
-/** 		  FRAME_PIXEL_HEIGHT (f) = height; **/
-		  f->win_gravity = NorthWestGravity;
+		  height = rect.bottom - rect.top;
+		  width = rect.right - rect.left;
+		  text_width = FRAME_PIXEL_TO_TEXT_WIDTH (f, width);
+		  text_height = FRAME_PIXEL_TO_TEXT_HEIGHT (f, height);
+		  rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (f, height);
+		  columns = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (f, width);
+
+		  /* TODO: Clip size to the screen dimensions.  */
+
+		  /* Even if the number of character rows and columns
+		     has not changed, the font size may have changed,
+		     so we need to check the pixel dimensions as well.  */
+
+		  if (width != FRAME_PIXEL_WIDTH (f)
+		      || height != FRAME_PIXEL_HEIGHT (f)
+		      || text_width != FRAME_TEXT_WIDTH (f)
+		      || text_height != FRAME_TEXT_HEIGHT (f))
+		    {
+		      change_frame_size (f, text_width, text_height, 0, 1, 0, 1);
+		      SET_FRAME_GARBAGED (f);
+		      cancel_mouse_face (f);
+		      /* Do we want to set these here ????  */
+		      /** 		FRAME_PIXEL_WIDTH (f) = width; **/
+		      /** 		FRAME_TEXT_WIDTH (f) = text_width; **/
+		      /** 		FRAME_PIXEL_HEIGHT (f) = height; **/
+		      f->win_gravity = NorthWestGravity;
+		    }
 		}
 	    }
 


--- src/dispnew.c~1	2014-08-17 07:29:32 +0300
+++ src/dispnew.c	2014-09-22 17:40:15 +0300
@@ -2139,8 +2139,11 @@ adjust_frame_glyphs_for_window_redisplay
 static void
 adjust_decode_mode_spec_buffer (struct frame *f)
 {
+  ssize_t frame_message_buf_size = FRAME_MESSAGE_BUF_SIZE (f);
+
+  eassert (frame_message_buf_size >= 0);
   f->decode_mode_spec_buffer = xrealloc (f->decode_mode_spec_buffer,
-					 FRAME_MESSAGE_BUF_SIZE (f) + 1);
+					 frame_message_buf_size + 1);
 }
 
 
@@ -5540,10 +5543,6 @@ change_frame_size_1 (struct frame *f, in
     {
       new_text_width = (new_width == 0) ? FRAME_TEXT_WIDTH (f) : new_width;
       new_text_height = (new_height == 0) ? FRAME_TEXT_HEIGHT (f) : new_height;
-      /* Consider rounding here: Currently, the root window can be
-	 larger than the frame in terms of columns/lines.  */
-      new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
-      new_lines = new_text_height / FRAME_LINE_HEIGHT (f);
     }
   else
     {
@@ -5556,6 +5555,12 @@ change_frame_size_1 (struct frame *f, in
   /* Compute width of windows in F.  */
   /* Round up to the smallest acceptable size.  */
   check_frame_size (f, &new_text_width, &new_text_height, 1);
+  /* Recompute the dimensions in character units, since
+     check_frame_size might have changed the pixel dimensions.  */
+  /* Consider rounding here: Currently, the root window can be
+     larger than the frame in terms of columns/lines.  */
+  new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
+  new_lines = new_text_height / FRAME_LINE_HEIGHT (f);
 
   /* This is the width of the frame without vertical scroll bars and
      fringe columns.  Do this after rounding - see discussion of




In GNU Emacs 24.3.93.1 (i686-pc-mingw32)
 of 2014-08-15 on HOME-C4E4A596F7
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --prefix=/d/usr --enable-checking=yes,glyphs 'CFLAGS=-Og
 -g3''

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x r e o p <backspace> <backspace> p o r t - e m <tab> 
<return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils time-date tooltip electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp
w32-common-fns disp-table w32-win w32-vars tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process w32notify w32
multi-tty emacs)

Memory information:
((conses 8 74217 7009)
 (symbols 32 17535 0)
 (miscs 32 33 127)
 (strings 16 10776 4344)
 (string-bytes 1 269654)
 (vectors 8 9550)
 (vector-slots 4 384749 6002)
 (floats 8 57 196)
 (intervals 28 237 95)
 (buffers 508 11))

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[martin rudalics]

 >    (gdb) p f->text_cols
 >    $6 = -3  <<<<<<<<<<<<<<<<<<<

What is the value of f->text_width here?

 >      (We also don't check errors returned by
 >      GetClientRect.)

Should we?  I wonder already why

	  if (f && !FRAME_ICONIFIED_P (f) && msg.msg.wParam != SIZE_MINIMIZED)

in w32_read_socket didn't catch this.

 >    . change_frame_size internally validates the requested dimensions,
 >      and doesn't allow them to become too small.  But it does that on
 >      pixel dimensions, and if those are corrected, the character-unit
 >      dimensions are not recalculated to reflect those corrections.

That's the bug.

 > +	      if (GetClientRect (msg.msg.hwnd, &rect)
 > +		  /* GetClientRect evidently returns (0, 0, 0, 0) if
 > +		     called on a minimized frame.  Such "dimensions"
 > +		     aren't useful anyway.  */
 > +		  && !(rect.bottom == 0
 > +		       && rect.top == 0
 > +		       && rect.left == 0
 > +		       && rect.right == 0))

It certainly can't harm to do that but I doubt whether it's worth to
include a similar change for the other platforms.

 > +  ssize_t frame_message_buf_size = FRAME_MESSAGE_BUF_SIZE (f);
 > +
 > +  eassert (frame_message_buf_size >= 0);

Good.  This part should definitely go to the trunk too.

 > +  /* Recompute the dimensions in character units, since
 > +     check_frame_size might have changed the pixel dimensions.  */
 > +  /* Consider rounding here: Currently, the root window can be
 > +     larger than the frame in terms of columns/lines.  */
 > +  new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
 > +  new_lines = new_text_height / FRAME_LINE_HEIGHT (f);

This part should fix the problem for all platforms.

Please check it in but please also make sure that only the changes in
adjust_decode_mode_spec_buffer and maybe those of w32_read_socket get
propagated to the trunk.  Did you verify that the trunk handles your
.emacs.desktop correctly?

Many thanks, martin

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

> Date: Mon, 22 Sep 2014 19:43:43 +0200
> From: martin rudalics <rudalics@gmx.at>
> 
>  >    (gdb) p f->text_cols
>  >    $6 = -3  <<<<<<<<<<<<<<<<<<<
> 
> What is the value of f->text_width here?

18, as expected (minimum width is 2 columns).

>  >      (We also don't check errors returned by
>  >      GetClientRect.)
> 
> Should we?

I certainly think so.  If GetClientRect fails, how does it make sense
to use what we find in the rectangle data structure we passed to it?
The values there are just garbage.

> I wonder already why
> 
> 	  if (f && !FRAME_ICONIFIED_P (f) && msg.msg.wParam != SIZE_MINIMIZED)
> 
> in w32_read_socket didn't catch this.

I have no idea, perhaps because we already made the frame visible, but
the window manager didn't yet have time to actually do so.  The fact
is we do get all-zero rectangle, I've seen that in the debugger.

>  > +	      if (GetClientRect (msg.msg.hwnd, &rect)
>  > +		  /* GetClientRect evidently returns (0, 0, 0, 0) if
>  > +		     called on a minimized frame.  Such "dimensions"
>  > +		     aren't useful anyway.  */
>  > +		  && !(rect.bottom == 0
>  > +		       && rect.top == 0
>  > +		       && rect.left == 0
>  > +		       && rect.right == 0))
> 
> It certainly can't harm to do that but I doubt whether it's worth to
> include a similar change for the other platforms.

I don't suggest such changes for other platforms, only for w32.

>  > +  /* Recompute the dimensions in character units, since
>  > +     check_frame_size might have changed the pixel dimensions.  */
>  > +  /* Consider rounding here: Currently, the root window can be
>  > +     larger than the frame in terms of columns/lines.  */
>  > +  new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
>  > +  new_lines = new_text_height / FRAME_LINE_HEIGHT (f);
> 
> This part should fix the problem for all platforms.
> 
> Please check it in but please also make sure that only the changes in
> adjust_decode_mode_spec_buffer and maybe those of w32_read_socket get
> propagated to the trunk.

The rest of the changes won't get propagated because the relevant code
in change_frame_size_1 is gone on the trunk.

> Did you verify that the trunk handles your .emacs.desktop correctly?

No, I didn't have a trunk binary on that machine.  But if the
validation of the frame dimensions on the tyrunk doesn't suffer from
this problem, the bug shouldn't happen.

Btw, the problem has nothing to do with my .emacs.desktop: the call to
w32_read_socket was from the loop where we wait for the frame to
become visible.  It can happen at any time with any desktop file that
re-creates more than one frame.

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[martin rudalics]

 > I certainly think so.  If GetClientRect fails, how does it make sense
 > to use what we find in the rectangle data structure we passed to it?
 > The values there are just garbage.

We have to check these values anyway because our window structure might
be too complex to fit into the rectangle returned by GetClientRect.  But
then we should probably also rewrite things like w32_clear_window as

   if (hdc && GetClientRect (FRAME_W32_WINDOW (f), &rect))
     w32_clear_rect (f, hdc, &rect);

 >>   > +  /* Recompute the dimensions in character units, since
 >>   > +     check_frame_size might have changed the pixel dimensions.  */
 >>   > +  /* Consider rounding here: Currently, the root window can be
 >>   > +     larger than the frame in terms of columns/lines.  */
 >>   > +  new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
 >>   > +  new_lines = new_text_height / FRAME_LINE_HEIGHT (f);

I never got around to ask you: Do you anywhere see a need to round up
the values of new_cols and new_lines in cases like this?

martin

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

> Date: Tue, 23 Sep 2014 07:48:40 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: bug-gnu-emacs@gnu.org
> 
>  > I certainly think so.  If GetClientRect fails, how does it make sense
>  > to use what we find in the rectangle data structure we passed to it?
>  > The values there are just garbage.
> 
> We have to check these values anyway because our window structure might
> be too complex to fit into the rectangle returned by GetClientRect.

Sorry, I don't think I understand this.  GetClientRect just returns
the dimensions of our frame, it doesn't know anything about Emacs
windows.

> But then we should probably also rewrite things like
> w32_clear_window as
> 
>    if (hdc && GetClientRect (FRAME_W32_WINDOW (f), &rect))
>      w32_clear_rect (f, hdc, &rect);

Indeed.

>  >>   > +  /* Recompute the dimensions in character units, since
>  >>   > +     check_frame_size might have changed the pixel dimensions.  */
>  >>   > +  /* Consider rounding here: Currently, the root window can be
>  >>   > +     larger than the frame in terms of columns/lines.  */
>  >>   > +  new_cols = new_text_width / FRAME_COLUMN_WIDTH (f);
>  >>   > +  new_lines = new_text_height / FRAME_LINE_HEIGHT (f);
> 
> I never got around to ask you: Do you anywhere see a need to round up
> the values of new_cols and new_lines in cases like this?

Yes, I think so.  I think the reason we didn't see any problems with
that is that GUI windows always over-allocate their glyph matrices, to
be prepared for dealing with the smallest possible font, which is
rarely if ever used.  But I think if you actually use that smallest
font for the default face, you will see the problem.

(Just make sure you don't round up for TTY frames, so as not to add
one extra row there.)

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[martin rudalics]

 >> We have to check these values anyway because our window structure might
 >> be too complex to fit into the rectangle returned by GetClientRect.
 >
 > Sorry, I don't think I understand this.  GetClientRect just returns
 > the dimensions of our frame, it doesn't know anything about Emacs
 > windows.

If all values returned by GetClientRect are zero, it's all to obvious
that our windows won't fit.  But if, for example, rect.right - rect.left
is too small to fit our windows, we face a similar same problem and have
to handle that anyway.  So I'm not sure whether we should separately
deal with the case where all rectangle values are zero.

 >> I never got around to ask you: Do you anywhere see a need to round up
 >> the values of new_cols and new_lines in cases like this?
 >
 > Yes, I think so.  I think the reason we didn't see any problems with
 > that is that GUI windows always over-allocate their glyph matrices, to
 > be prepared for dealing with the smallest possible font, which is
 > rarely if ever used.  But I think if you actually use that smallest
 > font for the default face, you will see the problem.

I thought it's not needed because in required_matrix_width we use the
pixel width when HAVE_WINDOW_SYSTEM is defined.

 > (Just make sure you don't round up for TTY frames, so as not to add
 > one extra row there.)

martin

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

> Date: Tue, 23 Sep 2014 18:15:49 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: bug-gnu-emacs@gnu.org
> 
>  >> We have to check these values anyway because our window structure might
>  >> be too complex to fit into the rectangle returned by GetClientRect.
>  >
>  > Sorry, I don't think I understand this.  GetClientRect just returns
>  > the dimensions of our frame, it doesn't know anything about Emacs
>  > windows.
> 
> If all values returned by GetClientRect are zero, it's all to obvious
> that our windows won't fit.  But if, for example, rect.right - rect.left
> is too small to fit our windows, we face a similar same problem and have
> to handle that anyway.  So I'm not sure whether we should separately
> deal with the case where all rectangle values are zero.

When they are zero, we know we shouldn't even call change_frame_size.

>  >> I never got around to ask you: Do you anywhere see a need to round up
>  >> the values of new_cols and new_lines in cases like this?
>  >
>  > Yes, I think so.  I think the reason we didn't see any problems with
>  > that is that GUI windows always over-allocate their glyph matrices, to
>  > be prepared for dealing with the smallest possible font, which is
>  > rarely if ever used.  But I think if you actually use that smallest
>  > font for the default face, you will see the problem.
> 
> I thought it's not needed because in required_matrix_width we use the
> pixel width when HAVE_WINDOW_SYSTEM is defined.

Is this a response to what's above or to what's below?

>  > (Just make sure you don't round up for TTY frames, so as not to add
>  > one extra row there.)

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[martin rudalics]

 >>   > Yes, I think so.  I think the reason we didn't see any problems with
 >>   > that is that GUI windows always over-allocate their glyph matrices, to
 >>   > be prepared for dealing with the smallest possible font, which is
 >>   > rarely if ever used.  But I think if you actually use that smallest
 >>   > font for the default face, you will see the problem.
 >>
 >> I thought it's not needed because in required_matrix_width we use the
 >> pixel width when HAVE_WINDOW_SYSTEM is defined.
 >
 > Is this a response to what's above or to what's below?

To what's above.  Currently we

       return (((WINDOW_PIXEL_WIDTH (w) + ch_width - 1)
	       / ch_width) * w->ncols_scale_factor
	      /* 2 partially visible columns in the text area.  */
	      + 2
	      /* One partially visible column at the right
		 edge of each marginal area.  */
	      + 1 + 1);

so columns aren't involved IIUC.

martin

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

> Date: Tue, 23 Sep 2014 21:17:02 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: bug-gnu-emacs@gnu.org
> 
>        return (((WINDOW_PIXEL_WIDTH (w) + ch_width - 1)
> 	       / ch_width) * w->ncols_scale_factor
> 	      /* 2 partially visible columns in the text area.  */
> 	      + 2
> 	      /* One partially visible column at the right
> 		 edge of each marginal area.  */
> 	      + 1 + 1);
> 
> so columns aren't involved IIUC.

I was talking about new_lines (you asked about both).

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

> Date: Tue, 23 Sep 2014 22:30:55 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 18528@debbugs.gnu.org
> 
> > Date: Tue, 23 Sep 2014 21:17:02 +0200
> > From: martin rudalics <rudalics@gmx.at>
> > CC: bug-gnu-emacs@gnu.org
> > 
> >        return (((WINDOW_PIXEL_WIDTH (w) + ch_width - 1)
> > 	       / ch_width) * w->ncols_scale_factor
> > 	      /* 2 partially visible columns in the text area.  */
> > 	      + 2
> > 	      /* One partially visible column at the right
> > 		 edge of each marginal area.  */
> > 	      + 1 + 1);
> > 
> > so columns aren't involved IIUC.
> 
> I was talking about new_lines

For which we do a similar calculation, I see.

So now I don't understand why you asked about rounding up, since we
evidently already do.

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[martin rudalics]

 > So now I don't understand why you asked about rounding up, since we
 > evidently already do.

Not everywhere.  For example, we don't in FRAME_MESSAGE_BUF_SIZE and
FRAME_CURSOR_X_LIMIT.  And the following check in compute_motion of
indent.c seems dubious as well.

	  if (!NILP (Vtruncate_partial_width_windows)
	      && (total_width < FRAME_COLS (XFRAME (WINDOW_FRAME (win)))))

martin

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

> Date: Wed, 24 Sep 2014 08:16:47 +0200
> From: martin rudalics <rudalics@gmx.at>
> CC: 18528@debbugs.gnu.org
> 
>  > So now I don't understand why you asked about rounding up, since we
>  > evidently already do.
> 
> Not everywhere.  For example, we don't in FRAME_MESSAGE_BUF_SIZE and
> FRAME_CURSOR_X_LIMIT.  And the following check in compute_motion of
> indent.c seems dubious as well.
> 
> 	  if (!NILP (Vtruncate_partial_width_windows)
> 	      && (total_width < FRAME_COLS (XFRAME (WINDOW_FRAME (win)))))

That has nothing to do with rounding, does it?

Anyway, FRAME_MESSAGE_BUF_SIZE has nothing to do with frame/window
width, AFAIR, it is just computed based on the width.

FRAME_CURSOR_X_LIMIT is used only for the echo area, so the
probability of having a non-default font there is very low.  We should
probably do that calculation in pixels, though.

As for the second example, I have a difficulty concocting a use case
when it should matter, due to a combination of conditions that enter
that block.  But feel free to fix that if it sometimes gives bad
results.

bug#18528: 24.3.93; Crash during restoration of frameset from desktop

[Eli Zaretskii]

I've committed the patches to fix this bug in r11751 on the emacs-24
branch, and I'm closing the bug.