Subject: Re: 24.3.93; Crash during restoration of frameset from desktop

From: martin rudalics <[email-masked]>
Date: Mon, 22 Sep 2014 19:43:43 +0200

>    (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