Re: [PATCH v2 4/6] vt/vt: Add SRG mouse reporting protocol

From: Jiri Slaby
Date: Thu Jul 02 2020 - 04:48:36 EST


On 01. 07. 20, 17:13, Tammo Block wrote:
> The SRG protocol indicates a button release by appending a "m" to the
> report. In this case the button number is not 3 (RELEASEEVENT) but
> the number of the button that was released. As release event are only
> reported for the first three buttons (LOWBUTTONMASK), we need to store
> the number on click events because it is not sent to us from userspace.
>
> We also need to check for the case where no button state change occurred
> at all (bit 6 set), in this case a value of 3 is OK even in SRG.
>
> Signed-off-by: Tammo Block <tammo.block@xxxxxxxxx>
> ---
> drivers/tty/vt/vt.c | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 9abf2829b1d3..9aae3eac7989 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -1838,15 +1838,34 @@ static inline void respond_ID(struct tty_struct *tty)
> respond_string(vt102_id, strlen(vt102_id), tty->port);
> }
>
> +#define ANYBUTTONMASK 0xc3
> +#define LOWBUTTONMASK 0x03

Insert _ before MASK.

> +#define RELEASEEVENT 0x03
> +
> enum { Mouse_X10 = 0, Mouse_SRG, Mouse_URXVT};

= 0 in unnecessary. And put one per line. And all capitals as
CodingStyle says.

You should name the enum somehow and use it as a type for
vc_protocol_mouse (you'd need a forward declaration of the enum in the
header).

> void mouse_report(struct tty_struct *tty, int butt, int mrx, int mry)
> {
> - char buf[8];
> + static char last_btn = RELEASEEVENT;
> + char buf[20];
> + bool rel;
> int len;
>
> - len = sprintf(buf, "\033[M%c%c%c", (char)(' ' + butt),
> - (char)('!' + mrx), (char)('!' + mry));
> + switch (vc_cons[fg_console].d->vc_protocol_mouse) {
> + case Mouse_SRG:

This is not how we indent switch-case.

> + rel = (butt & ANYBUTTONMASK) == RELEASEEVENT;
> + if ((butt & ANYBUTTONMASK) < RELEASEEVENT)
> + last_btn = butt & LOWBUTTONMASK;
> + if ((butt & TIOCL_SELBUTTONMASK) == RELEASEEVENT)
> + butt = (butt & ~LOWBUTTONMASK) | last_btn;
> + len = sprintf(buf, "\033[<%d;%d;%d%c", butt,
> + mrx + 1, mry + 1, rel ? 'm' : 'M');
> + break;
> + default:
> + len = sprintf(buf, "\033[M%c%c%c", (char)(' ' + butt),
> + (char)('!' + mrx), (char)('!' + mry));
> + break;
> + }
> respond_string(buf, len, tty->port);
> }

thanks,
--
js
suse labs