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

[Jiri Slaby <jirislaby@kernel.org>]

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@gmail.com>
> ---
>  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