[PATCH] fix broken mouse in Cygwin 1.7.14 and 1.7.15

[Thomas Wolff]

Am 17.05.2012 04:19, schrieb Christopher Faylor:
> On Thu, May 17, 2012 at 01:20:06AM +0200, Mikulas Patocka wrote:
>> 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.
> I don't know where that odd code came from.  It's apparently been in
> smallprint.{c,cc} forever.  I'm nuking it.  I see one potential problem
> in fhandler_socket::bind.  It won't be hard to work around if that
> function was really relying on it.
>
> Thanks for tracking this down.  The change will be in the next snapshot.
And thanks for analyzing this show-stopper for my mouse reporting patch; 
I should have noticed myself...
Thomas

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