[PATCH v2 0/6] vt: Add SRG mouse reporting features

[PATCH v2 0/6] vt: Add SRG mouse reporting features

[Tammo Block]

Hi everybody,

this patchset adds xterm like mouse reporting features to the console.

The linux virtual console has support for mouse reporting since 1994 or so,
but the kernel only supports the original X10/X11 style standard protocols.
To support more protocols these patches expand the kernel structures in a
up- and downwards compatible way, see the last patch for detailed
documentation and pointers to even more detailed docs.

The main goal is to become compatible with xterm, as most TUI software today
is tested in xterm or another compatible terminal.

Support by the mouse daemons (consolation, gpm) will be needed too.


Kind regards,
Tammo

Changes from v1:
- Really fixed the style und spelling errors (Sorry Randy!)
- Created defines and enums for better readability
- Made variable to store last pressed button static and moved into
  mouse_report function  

Changes from v0:
- Fixed al the style the things mentioned by Jiri and Randy (thanks!)
- Change datastructure for report (better compatibility)
- Changed documentation in large parts accordingly
- Added URXVT protocol


Tammo Block (6):
  tiocl.h: Change/Add defines for mouse report
  console_struct.h: Two members for mouse report
  vt/vt: Enable mode change via escape sequence
  vt/vt: Add SRG mouse reporting protocol
  vt/vt: Add URXVT mouse reporting protocol
  Documentation: Describe console mouse reporting

 .../admin-guide/console-mouse-reporting.rst   | 88 +++++++++++++++++++
 Documentation/admin-guide/index.rst           |  1 +
 drivers/tty/vt/vt.c                           | 47 ++++++++--
 include/linux/console_struct.h                |  3 +-
 include/uapi/linux/tiocl.h                    |  8 +-
 5 files changed, 139 insertions(+), 8 deletions(-)
 create mode 100644 Documentation/admin-guide/console-mouse-reporting.rst

-- 
2.27.0

[PATCH v2 1/6] tiocl.h: Change/Add defines for mouse report

[Tammo Block]

Add additional defines for mouse event types. The change of the value
of TIOCL_SELBUTTONMASK deserves a bit more explanation :

The old value of 15 uses the first 4 bits and sends them unchanged back
to userspace if requested by an application. But in fact only the first
two bits have ever been used by any daemon or useful at all, as the
kernel already knows the status of the shift and alt keys encoded in
bits 3 and 4. On the other hand we *do* want to know the status of bits
6-8, encoding button values >3 and mouse move and drag operations.

This change is up- and downwards compatible by masking all spourious
bits and leaving only the undisputed parts (bits 1 and 2) untouched.

Signed-off-by: Tammo Block <tammo.block@gmail.com>
---
 include/uapi/linux/tiocl.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/tiocl.h b/include/uapi/linux/tiocl.h
index b32acc229024..3717e865571d 100644
--- a/include/uapi/linux/tiocl.h
+++ b/include/uapi/linux/tiocl.h
@@ -9,7 +9,7 @@
 #define 	TIOCL_SELPOINTER	3	/* show the pointer */
 #define 	TIOCL_SELCLEAR	4	/* clear visibility of selection */
 #define 	TIOCL_SELMOUSEREPORT	16	/* report beginning of selection */
-#define 	TIOCL_SELBUTTONMASK	15	/* button mask for report */
+#define 	TIOCL_SELBUTTONMASK	0xe3	/* button mask for report */
 /* selection extent */
 struct tiocl_selection {
 	unsigned short xs;	/* X start */
@@ -28,7 +28,11 @@ struct tiocl_selection {
 
 /* these two don't return a value: they write it back in the type */
 #define TIOCL_GETSHIFTSTATE	6	/* write shift state */
-#define TIOCL_GETMOUSEREPORTING	7	/* write whether mouse event are reported */
+#define TIOCL_GETMOUSEREPORTING	7	/* write which mouse events are reported */
+#define		TIOCL_REPORTBTNPRESS	1	/* report button press only    "9" */
+#define		TIOCL_REPORTRELEASE	2	/* report press and release "1000" */
+#define		TIOCL_REPORTDRAG	3	/* report drag events       "1002" */
+#define		TIOCL_REPORTANYMOVE	4	/* report any movement      "1003" */
 #define TIOCL_SETVESABLANK	10	/* set vesa blanking mode */
 #define TIOCL_SETKMSGREDIRECT	11	/* restrict kernel messages to a vt */
 #define TIOCL_GETFGCONSOLE	12	/* get foreground vt */
-- 
2.27.0

[PATCH v2 2/6] console_struct.h: Two members for mouse report

[Tammo Block]

We need two values to store the status of mouse reporting, both need at
least two (vc_protocol_mouse) or three (vc_report_mouse) bits, so use
chars.

Signed-off-by: Tammo Block <tammo.block@gmail.com>
---
 include/linux/console_struct.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h
index 153734816b49..dd42287ed553 100644
--- a/include/linux/console_struct.h
+++ b/include/linux/console_struct.h
@@ -132,6 +132,8 @@ struct vc_data {
 	struct pid 	*vt_pid;
 	int		vt_newvt;
 	wait_queue_head_t paste_wait;
+	unsigned char	vc_report_mouse;	/* Which events to report to userspace */
+	unsigned char	vc_protocol_mouse;	/* What protocol to use for report */
 	/* mode flags */
 	unsigned int	vc_disp_ctrl	: 1;	/* Display chars < 32? */
 	unsigned int	vc_toggle_meta	: 1;	/* Toggle high bit? */
@@ -144,7 +146,6 @@ struct vc_data {
 	unsigned int	vc_priv		: 3;
 	unsigned int	vc_need_wrap	: 1;
 	unsigned int	vc_can_do_color	: 1;
-	unsigned int	vc_report_mouse : 2;
 	unsigned char	vc_utf		: 1;	/* Unicode UTF-8 encoding */
 	unsigned char	vc_utf_count;
 		 int	vc_utf_char;
-- 
2.27.0

[PATCH v2 3/6] vt/vt: Enable mode change via escape sequence

[Tammo Block]

This enables userspace to enable one of the mouse protocols and choose
one of the new event types by escape sequences.

And don't forget to reset protocol value also if resetting vc.

Signed-off-by: Tammo Block <tammo.block@gmail.com>
---
 drivers/tty/vt/vt.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 673177d4e859..9abf2829b1d3 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1838,6 +1838,8 @@ static inline void respond_ID(struct tty_struct *tty)
 	respond_string(vt102_id, strlen(vt102_id), tty->port);
 }
 
+enum { Mouse_X10 = 0, Mouse_SRG, Mouse_URXVT};
+
 void mouse_report(struct tty_struct *tty, int butt, int mrx, int mry)
 {
 	char buf[8];
@@ -1896,13 +1898,25 @@ static void set_mode(struct vc_data *vc, int on_off)
 					clr_kbd(vc, decarm);
 				break;
 			case 9:
-				vc->vc_report_mouse = on_off ? 1 : 0;
+				vc->vc_report_mouse = on_off * TIOCL_REPORTBTNPRESS;
 				break;
 			case 25:		/* Cursor on/off */
 				vc->vc_deccm = on_off;
 				break;
 			case 1000:
-				vc->vc_report_mouse = on_off ? 2 : 0;
+				vc->vc_report_mouse = on_off * TIOCL_REPORTRELEASE;
+				break;
+			case 1002:
+				vc->vc_report_mouse = on_off * TIOCL_REPORTDRAG;
+				break;
+			case 1003:
+				vc->vc_report_mouse = on_off * TIOCL_REPORTANYMOVE;
+				break;
+			case 1006:
+				vc->vc_protocol_mouse = on_off * Mouse_SRG;
+				break;
+			case 1015:
+				vc->vc_protocol_mouse = on_off * Mouse_URXVT;
 				break;
 			}
 		} else {
@@ -2067,6 +2081,7 @@ static void reset_terminal(struct vc_data *vc, int do_clear)
 	vc->state.charset	= 0;
 	vc->vc_need_wrap	= 0;
 	vc->vc_report_mouse	= 0;
+	vc->vc_protocol_mouse	= 0;
 	vc->vc_utf              = default_utf8;
 	vc->vc_utf_count	= 0;
 
-- 
2.27.0

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

[Tammo Block]

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
+#define RELEASEEVENT	0x03
+
 enum { Mouse_X10 = 0, Mouse_SRG, Mouse_URXVT};
 
 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:
+			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);
 }
 
-- 
2.27.0

[PATCH v2 5/6] vt/vt: Add URXVT mouse reporting protocol

[Tammo Block]

The URXVT protocol easy, all data analog to the old X10.

Signed-off-by: Tammo Block <tammo.block@gmail.com>
---
 drivers/tty/vt/vt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9aae3eac7989..33e43cbfa1fc 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1861,6 +1861,9 @@ void mouse_report(struct tty_struct *tty, int butt, int mrx, int mry)
 			len = sprintf(buf, "\033[<%d;%d;%d%c", butt,
 					mrx + 1, mry + 1, rel ? 'm' : 'M');
 			break;
+		case Mouse_URXVT:
+			len = sprintf(buf, "\033[%d;%d;%dM", butt + 32, mrx + 1, mry + 1);
+			break;
 		default:
 			len = sprintf(buf, "\033[M%c%c%c", (char)(' ' + butt),
 					(char)('!' + mrx), (char)('!' + mry));
-- 
2.27.0

[PATCH v2 6/6] Documentation: Describe console mouse reporting

[Tammo Block]

This patch adds a description of the kernel interface(s) used for vt
console mouse reporting and describes the protocols and bitmasks.

Signed-off-by: Tammo Block <tammo.block@gmail.com>
---
 .../admin-guide/console-mouse-reporting.rst   | 88 +++++++++++++++++++
 Documentation/admin-guide/index.rst           |  1 +
 2 files changed, 89 insertions(+)
 create mode 100644 Documentation/admin-guide/console-mouse-reporting.rst

diff --git a/Documentation/admin-guide/console-mouse-reporting.rst b/Documentation/admin-guide/console-mouse-reporting.rst
new file mode 100644
index 000000000000..c75a627f27b8
--- /dev/null
+++ b/Documentation/admin-guide/console-mouse-reporting.rst
@@ -0,0 +1,88 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=======================
+Console Mouse Reporting
+=======================
+
+A terminal may send escape sequences to enable applications to react to mouse
+input. As the kernel does not know when to emit these events a mouse daemon
+is needed to react to mouse movements and signal the kernel accordingly. The
+kernel will then send an escape sequence to the application. This is called
+mouse reporting and several types and protocols have been developed over time.
+
+See tiocl.h, the :manpage:`ioctl_console(2)` and :manpage:`console_codes(4)`
+man pages and the xterm [1]_ or terminalguide [2]_ home pages for a detailed
+list and description of the various protocols, their bit layout as well as
+their limitations.
+
+Events and formats
+++++++++++++++++++
+
+A Linux console keeps state about two different aspects of mouse reporting,
+the kind of **events** to be reported and the **format** to send to userspace.
+
+A mouse daemon can check which kind of mouse events a client wants to be
+informed about via the TIOCLINUX ioctl, using the TIOCL_GETMOUSEREPORTING
+subcall. The values of the supported event classes (9, 1000, 1002, 1003) are
+described in tiocl.h. Based on this information the daemon is responsible
+for not sending data packages for unrequested events.
+
+A userspace client may request to be informed by the kernel about one of
+the event classes and choose one of the data formats URXVT (1005), SRG
+(1006) or X10/X11 (default) via console escape sequences. In general all
+of them encode similar information, only the escape sequences differ.
+
+See the xterm [1]_ or terminalguide [2]_ home pages for all details.
+
+Reports from kernel to userspace client
++++++++++++++++++++++++++++++++++++++++
+
+The requested events are sent by the kernel to userspace encoded in an
+escape sequences; details depend on the chosen format. All of them use one
+based pointer coordinates and a single byte to encode the button status.
+
+Short summary (we call this the SRG button format for the rest of this text):
+
+ - 1,2 : Buttons, lower bits (see notes below)
+ - 3-5 : Modifier keys (Shift, Alt and Ctrl)
+ - 6   : Mouse movement only, no button status change
+ - 7-8 : Buttons, upper bits (for buttons 4-15)
+
+Reports sent from daemon to kernel
+++++++++++++++++++++++++++++++++++
+
+A report is sent by a mouse daemon to the kernel via the TIOCLINUX ioctl,
+using the TIOCL_SETSEL subcall. The coordinates are encoded zero based in
+xs and ys, with 0,0 as the upper left corner, but see the note below.
+The format used by the userspace mouse daemon for button encoding is almost
+identical to the SRG button layout described above and is put into the sel_mode
+of the tiocl_selection struct. All bits masked in TIOCL_SELBUTTONMASK are
+unchanged compared to the SRG button format above; the remaining three are
+changed the following way:
+
+- 3,4  : Unused, must be zero. The kernel knows modifier key state anyway.
+- 5    : Always 1, identifies mouse report / TIOCL_SELMOUSEREPORT
+
+Notes
++++++
+
+Button numbers are encoded like this:
+
+- 0-2  : Left, middle and right button
+- 3    : No button pressed / Button release
+- 4-15 : More buttons, e.g. 4 and 5 are scroll wheel
+
+Please note that button releases should only be reported for buttons 0-2.
+
+Also note that coordinates (xs,ys,xe,ye) are zero based for the TIOCL_SETSEL
+syscall but one based for the escape sequences sent by the kernel, so the
+kernel will increase all coordinates by one.
+
+Older kernels only used the lower 4 bits of sel_mode, effectively limiting
+the protocol to 3 buttons and button click only. The meaning of the 4 bits
+is equivalent to the SRG button layout. Note that newer kernels will ignore
+the upper two bits (modifier keys).
+
+.. [1] https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking
+.. [2] https://terminalguide.namepad.de/mouse/
+
diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst
index 58c7f9fc2396..c535902f3851 100644
--- a/Documentation/admin-guide/index.rst
+++ b/Documentation/admin-guide/index.rst
@@ -71,6 +71,7 @@ configure specific aspects of kernel behavior to your liking.
    cgroup-v2
    cifs/index
    clearing-warn-once
+   console-mouse-reporting
    cpu-load
    cputopology
    dell_rbu
-- 
2.27.0

Re: [PATCH v2 6/6] Documentation: Describe console mouse reporting

[Randy Dunlap]

On 7/1/20 8:14 AM, Tammo Block wrote:
> This patch adds a description of the kernel interface(s) used for vt
> console mouse reporting and describes the protocols and bitmasks.
> 
> Signed-off-by: Tammo Block <tammo.block@gmail.com>
> ---
>  .../admin-guide/console-mouse-reporting.rst   | 88 +++++++++++++++++++
>  Documentation/admin-guide/index.rst           |  1 +
>  2 files changed, 89 insertions(+)
>  create mode 100644 Documentation/admin-guide/console-mouse-reporting.rst
> 
> diff --git a/Documentation/admin-guide/console-mouse-reporting.rst b/Documentation/admin-guide/console-mouse-reporting.rst
> new file mode 100644
> index 000000000000..c75a627f27b8
> --- /dev/null
> +++ b/Documentation/admin-guide/console-mouse-reporting.rst
> @@ -0,0 +1,88 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=======================
> +Console Mouse Reporting
> +=======================
> +
> +A terminal may send escape sequences to enable applications to react to mouse
> +input. As the kernel does not know when to emit these events a mouse daemon
> +is needed to react to mouse movements and signal the kernel accordingly. The
> +kernel will then send an escape sequence to the application. This is called
> +mouse reporting and several types and protocols have been developed over time.
> +
> +See tiocl.h, the :manpage:`ioctl_console(2)` and :manpage:`console_codes(4)`
> +man pages and the xterm [1]_ or terminalguide [2]_ home pages for a detailed
> +list and description of the various protocols, their bit layout as well as
> +their limitations.
> +
> +Events and formats
> +++++++++++++++++++
> +
> +A Linux console keeps state about two different aspects of mouse reporting,
> +the kind of **events** to be reported and the **format** to send to userspace.
> +
> +A mouse daemon can check which kind of mouse events a client wants to be
> +informed about via the TIOCLINUX ioctl, using the TIOCL_GETMOUSEREPORTING
> +subcall. The values of the supported event classes (9, 1000, 1002, 1003) are
> +described in tiocl.h. Based on this information the daemon is responsible
> +for not sending data packages for unrequested events.
> +
> +A userspace client may request to be informed by the kernel about one of
> +the event classes and choose one of the data formats URXVT (1005), SRG
> +(1006) or X10/X11 (default) via console escape sequences. In general all
> +of them encode similar information, only the escape sequences differ.
> +
> +See the xterm [1]_ or terminalguide [2]_ home pages for all details.
> +
> +Reports from kernel to userspace client
> ++++++++++++++++++++++++++++++++++++++++
> +
> +The requested events are sent by the kernel to userspace encoded in an
> +escape sequences; details depend on the chosen format. All of them use one

          sequence;

(this word was correct in v1)

> +based pointer coordinates and a single byte to encode the button status.
> +
> +Short summary (we call this the SRG button format for the rest of this text):
> +
> + - 1,2 : Buttons, lower bits (see notes below)
> + - 3-5 : Modifier keys (Shift, Alt and Ctrl)
> + - 6   : Mouse movement only, no button status change
> + - 7-8 : Buttons, upper bits (for buttons 4-15)
> +
> +Reports sent from daemon to kernel
> +++++++++++++++++++++++++++++++++++
> +
> +A report is sent by a mouse daemon to the kernel via the TIOCLINUX ioctl,
> +using the TIOCL_SETSEL subcall. The coordinates are encoded zero based in
> +xs and ys, with 0,0 as the upper left corner, but see the note below.
> +The format used by the userspace mouse daemon for button encoding is almost
> +identical to the SRG button layout described above and is put into the sel_mode
> +of the tiocl_selection struct. All bits masked in TIOCL_SELBUTTONMASK are
> +unchanged compared to the SRG button format above; the remaining three are
> +changed the following way:
> +
> +- 3,4  : Unused, must be zero. The kernel knows modifier key state anyway.
> +- 5    : Always 1, identifies mouse report / TIOCL_SELMOUSEREPORT
> +
> +Notes
> ++++++
> +
> +Button numbers are encoded like this:
> +
> +- 0-2  : Left, middle and right button
> +- 3    : No button pressed / Button release
> +- 4-15 : More buttons, e.g. 4 and 5 are scroll wheel
> +
> +Please note that button releases should only be reported for buttons 0-2.
> +
> +Also note that coordinates (xs,ys,xe,ye) are zero based for the TIOCL_SETSEL
> +syscall but one based for the escape sequences sent by the kernel, so the
> +kernel will increase all coordinates by one.
> +
> +Older kernels only used the lower 4 bits of sel_mode, effectively limiting
> +the protocol to 3 buttons and button click only. The meaning of the 4 bits
> +is equivalent to the SRG button layout. Note that newer kernels will ignore
> +the upper two bits (modifier keys).
> +
> +.. [1] https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking
> +.. [2] https://terminalguide.namepad.de/mouse/
> +

thanks.
-- 
~Randy

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

[Jiri Slaby]

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

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

[Tammo Block]

Hi Jiri,

thanks for your patience ... ;-)

Am Do., 2. Juli 2020 um 10:48 Uhr schrieb 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.
>

OK, I was just copying the style of other enums in vt.c.
But the code seems to be older than the coding style guide ... ;-)

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

That would make vc_protocol_mouse an enum and vc_report_mouse a define.
As vc_report_mouse is visible from userspace those defines live in tiocl.h.

Wouldn't it be more consistent to make both of them the same type and also use
defines for vc_report_mouse? And if so: Where to place them?

They are only needed inside vt.c, so they could live in any of:
1.) Inside vt.c
2.) in vt.h
3.) in tiocl.h, although userspace does not need them.

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

Thanks again,
Tammo