This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] fix broken mouse in Cygwin 1.7.14 and 1.7.15



On Thu, 17 May 2012, Mikulas Patocka wrote:

> > Corinna Vinschen:
> >
> > > 2012-04-03  Thomas Wolff
> > > 
> > > 	* fhandler.h (class dev_console): Two flags for extended mouse modes.
> > > 	* fhandler_console.cc (fhandler_console::read): Implemented 
> > > 	extended mouse modes 1015 (urxvt, mintty, xterm) and 1006 (xterm).
> > > 	Not implemented extended mouse mode 1005 (xterm, mintty).
> > > 	Supporting mouse coordinates greater than 222 (each axis).
> > > 	Also: two { wrap formatting consistency fixes.
> > > 	(fhandler_console::char_command) Initialization of enhanced 
> > > 	mouse reporting modes.
> > 
> > 
> > Patch applied with changes.  Please use __small_sprintf rather than
> > sprintf.  I also changed the CHangeLog entry slightly.  Keep it short
> > and in present tense.
> 
> Hi
> 
> The change (sprintf -> __small_sprintf) that Corinna applied actually 
> broke mouse reporting. It is broken in Cygwin 1.7.14 and 1.7.15.
> 
> When the user clicks with the first button, the mouse down event is 
> reported incorrectly, the mouse down event always looks like this:
>  1b 5b([) 4d(M) 30(0) 78(x) 32(2)
> Note that there is 0x30 (instead of 0x20) as the button. And there are 
> always fixed coordinates (0x78, 0x32), regardless of where the user 
> clicks.
> 
> This bug breaks the Links textmode browser (if you click on the top line, 
> you should see the menu, but you don't with Cygwin 1.7.14 and 1.7.15). It 
> also break Midnight Commander (if you start it with "TERM=xterm mc") and 
> all other programs that use xterm-style mouse reporting.
> 
> The reason is that __small_sprintf and sprintf aren't equivalent. 
> __small_sprintf processes '%c' format string differently from sprintf. A 
> piece of code from smallprint.cc:
> 
> case 'c':
>   {
>     int c = va_arg (ap, int);
>     if (c > ' ' && c <= 127)
>       *dst++ = c;
>     else
>       {
>         *dst++ = '0';
>         *dst++ = 'x';
>         dst = __rn (dst, 16, 0, c, len, pad, LMASK);
>       }
>   }
>   break;
> 
> We see that if the character is outside the range 0x21..0x7f, 
> __small_sprintf prints 0x and the hex value. On the other hand, sprintf 
> copies the byte unchanged.
> 
> __small_sprintf("%c", 32) doesn't print space, it prints 0x20 --- and this 
> breaks mouse click reporting. It also breaks the extended coordinate 
> reporting implemented by Thomas because it need to print characters >= 
> 0x80.
> 
> The attached patch fixes the mouse bug by changing __small_sprintf back to 
> sprintf. Alternatively, you can fix __small_sprintf to process "%c" 
> consistently with sprintf, but I don't know how much other code is 
> dependent on the current peculiar "%c" processing. So it's safer to change 
> mouse reporting calls to use sprintf.
> 
> Mikulas
> 
> ---
> 
> --- cygwin-1.7.14-2/winsup/cygwin/fhandler_console.cc_	2012-05-17 00:38:55.790039000 +0200
> +++ cygwin-1.7.14-2/winsup/cygwin/fhandler_console.cc	2012-05-17 00:44:18.529296800 +0200
> @@ -625,17 +625,17 @@
>  		/* We can now create the code. */
>  		if (dev_state.ext_mouse_mode6)
>  		  {
> -		    __small_sprintf (tmp, "\033[<%d;%d;%d%c", b,
> -				     dev_state.dwMousePosition.X + 1,
> -				     dev_state.dwMousePosition.Y + 1,
> -				     mode6_term);
> +		    sprintf (tmp, "\033[<%d;%d;%d%c", b,
> +			     dev_state.dwMousePosition.X + 1,
> +			     dev_state.dwMousePosition.Y + 1,
> +			     mode6_term);
>  		    nread = strlen (tmp);
>  		  }
>  		else if (dev_state.ext_mouse_mode15)
>  		  {
> -		    __small_sprintf (tmp, "\033[%d;%d;%dM", b + 32,
> -				     dev_state.dwMousePosition.X + 1,
> -				     dev_state.dwMousePosition.Y + 1);
> +		    sprintf (tmp, "\033[%d;%d;%dM", b + 32,
> +			     dev_state.dwMousePosition.X + 1,
> +			     dev_state.dwMousePosition.Y + 1);
>  		    nread = strlen (tmp);
>  		  }
>  		else if (dev_state.ext_mouse_mode5)
> @@ -643,7 +643,7 @@
>  		    unsigned int xcode = dev_state.dwMousePosition.X + ' ' + 1;
>  		    unsigned int ycode = dev_state.dwMousePosition.Y + ' ' + 1;
>  
> -		    __small_sprintf (tmp, "\033[M%c", b + ' ');
> +		    sprintf (tmp, "\033[M%c", b + ' ');
>  		    nread = 4;
>  		    /* the neat nested encoding function of mintty 
>  		       does not compile in g++, so let's unfold it: */
> @@ -674,8 +674,8 @@
>  		      xcode = 0;
>  		    if (ycode >= 256)
>  		      ycode = 0;
> -		    __small_sprintf (tmp, "\033[M%c%c%c", b + ' ',
> -				     xcode, ycode);
> +		    sprintf (tmp, "\033[M%c%c%c", b + ' ',
> +			     xcode, ycode);
>  		    nread = 6;	/* tmp may contain NUL bytes */
>  		  }
>  		syscall_printf ("mouse: %s at (%d,%d)", sz,
> @@ -691,9 +691,9 @@
>  	  if (dev_state.use_focus)
>  	    {
>  	      if (input_rec.Event.FocusEvent.bSetFocus)
> -	        __small_sprintf (tmp, "\033[I");
> +	        sprintf (tmp, "\033[I");
>  	      else
> -	        __small_sprintf (tmp, "\033[O");
> +	        sprintf (tmp, "\033[O");
>  
>  	      toadd = tmp;
>  	      nread = 3;
> 

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]