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